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

feat: Create CustomList #85

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

feat: Create CustomList #85

wants to merge 2 commits into from

Conversation

RemiBardon
Copy link
Member

@RemiBardon RemiBardon commented Jul 26, 2022

Following a Slack discussion on #78 (comment), we decided to reimplement a basic SwiftUI List using a VStack.

I managed to recreate a good portion of the List API, or at least what we will need of it.

Here are the results:

Screenshot 2022-07-26 at 10 40 03

When the window is not key:

Screenshot 2022-07-26 at 10 40 12

With sections (supports .headerProminence(.increased)):

Screenshot 2022-07-26 at 10 41 02

I had to create a custom selection shape, to get the same effect as List when multiple items are selected and touch each other:

Screenshot 2022-07-26 at 10 44 04

There is no support for keyboard navigation. This must be implemented higher in the hierarchy, or I can implement it here as well if needed. The higher implementation will be necessary anyway, when the list is not first responder, which is the only place where we will use CustomList, so I don't think we need to duplicate this code.


Initial TODO list from #78:

  • Accept a view for its cells provided from outside
  • Bind its selection to the ViewStore
  • Handle loading, error and empty states
  • Its state offers convenience methods to manipulate the list selection

@RemiBardon RemiBardon requested a review from nesium July 26, 2022 08:48
@RemiBardon RemiBardon self-assigned this Jul 26, 2022
@RemiBardon RemiBardon force-pushed the custom-list branch 2 times, most recently from ac42c2e to 7151f6a Compare July 26, 2022 08:50
@RemiBardon
Copy link
Member Author

I'll just add an initializer which takes sectionned content but a single selection, I need it for #78.

@RemiBardon
Copy link
Member Author

RemiBardon commented Jul 26, 2022

I just tested with the attached window, and it works (using .fixedSize(horizontal: true, vertical: false) for rows and header. I did not enforce this in my CustomList implementation, should I? It would prevent us from creating rows with content on both sides… which is very unfortunate. I couldn't find a workaround… yet.

Also, the attached window is not key, so it gives this bad result. However, this must be fixed in the other PR, and https://github.com/smic/SuggestionsDemo seems to handle it well. We'll see.

Screenshot 2022-07-26 at 15 08 56

Screenshot 2022-07-26 at 15 10 36

Screenshot 2022-07-26 at 15 10 50

@RemiBardon
Copy link
Member Author

https://github.com/smic/SuggestionsDemo seems to handle it well.

No it does not. I changed a few things to use controlActiveState, and it has the same problem. The text field is first responder, I don't think we can solve this issue.

Screenshot 2022-07-26 at 15 35 47

I will have to add some environment value or value in the initializer to force the usage of .fill(.tint), as the attached window will disappear when the main window is not key anyway.

@RemiBardon
Copy link
Member Author

The magic of SwiftUI: I just added .environment(\.controlActiveState, .active) to the attached window, and it works 🥳

Screenshot 2022-07-26 at 15 40 50

@nesium Should I add this modifier on AttachedWindow itself or are there cases where we won't want to enforce this?

@RemiBardon RemiBardon marked this pull request as draft August 17, 2022 09:13
@RemiBardon RemiBardon removed their assignment May 15, 2023
@RemiBardon RemiBardon added the stale Not worked on for a long time label May 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Not worked on for a long time
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant