Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds custom spacing to lists #352

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

icopano
Copy link

@icopano icopano commented Nov 12, 2023

Added the possibility to setup a custom spacing to List.
I saw this in a closed issue which was never addressed and thought It would be a nice addition.
This implementation assumes the use of a constant spacing.

Copy link

codecov bot commented Nov 17, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (b7c7e7e) 78.86% compared to head (2b49646) 78.86%.
Report is 2 commits behind head on main.

Files Patch % Lines
Source/Internal/List/PDFListObject.swift 80.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #352   +/-   ##
=======================================
  Coverage   78.86%   78.86%           
=======================================
  Files         212      212           
  Lines       12607    12617   +10     
=======================================
+ Hits         9942     9950    +8     
- Misses       2665     2667    +2     
Flag Coverage Δ
iOS 79.77% <80.00%> (+<0.01%) ⬆️
macOS 0.00% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@philprime
Copy link
Member

Hi @icopano! Sorry for my super late response, busy times and this got lost in my inbox.

Your implementation looks straight away, but I will adapt it slightly, so the flatted() method to find the last element isn't called on every single loop.

@philprime
Copy link
Member

philprime commented Apr 25, 2024

Ok it looks like I can't push to your branch because of branch protections of the main branch (it's often easier to create a new branch after forking).

Instead I pushed my recommended changes in the branch issue-352-adaptions, and I would kindly request you to adapt your PR according to it. I also merged main into it, because I did a lot of changes in the last weeks, and there is a merge conflict.

Commit with my changes: 730bcf3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants