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

Feature/segmented picker animation #10

Merged
merged 34 commits into from
Nov 15, 2024
Merged

Conversation

r-jarvis
Copy link
Collaborator

@r-jarvis r-jarvis commented Nov 8, 2024

BetterPickerStyle that mimics SwiftUI PickerStyle.segmented

Explanation for changes made outside of SegmentedBetterPickerStyle

  • BetterPickerStyleListCellConfiguration.select and deselect were made public. The reasoning for this was that with a .segmented style, it does not necessarily make sense to "toggle" when selecting. These normally have a default value selected to start and do not allow for a case where nothing is selected.
  • Added selectionIndexSet to BetterPickerStyleConfiguration. This was needed because of the way we needed to handle the UI. We need to know which selection is chosen in order to move the View that displays the selection. We have to do this outside of the makeListCell since we want the View to animate between cells when changing our selection.

Explanation of some decisions made for SegmentedBetterPickerStyle

  • I could not think of a way to get the selection animation working without requiring a width for the BetterPickerStyle. I tried doing this using GeometryReader, but that also requires a width or it will expand to the entire parent View (not ideal). It would be great if SegmentedBetterPickerStyle had an optional frameWidth, and if that value was nil it would auto-size. I could not get that working though.
  • For dark mode, we use the same UI as we use for light mode. The reason for this is @Environment(\.colorScheme) var colorScheme was not working properly for me. Maybe I was doing something wrong, but it was not changing based on the selected Color Scheme in Previewer. While not optimal, everything is still very much usable in both light and dark modes.
  • I added a few configurable parameters to the style: frameWidth, frameHeight and horizontalCellAlignment. My thought was that these would be nice to modify at the parent level. Any cell-based styling should be able to be handled via the ItemContent. Let me know if there are more configurable style options you'd like to see.

@r-jarvis r-jarvis marked this pull request as draft November 8, 2024 16:58
@r-jarvis r-jarvis changed the title Feature/segmented picker animation Feature/segmented picker animation (Relies on PR #12) Nov 13, 2024
@r-jarvis r-jarvis marked this pull request as ready for review November 13, 2024 14:02
@r-jarvis r-jarvis marked this pull request as draft November 13, 2024 14:02
@r-jarvis r-jarvis changed the title Feature/segmented picker animation (Relies on PR #12) Feature/segmented picker animation Nov 13, 2024
@r-jarvis r-jarvis marked this pull request as ready for review November 13, 2024 20:30
@roanutil
Copy link
Member

This is very minor, but did you consider adding dividers between the segments?

@roanutil
Copy link
Member

roanutil commented Nov 14, 2024

I'm also unable to get colorScheme to work. I'll create an issue for that.

#13

@r-jarvis
Copy link
Collaborator Author

This is very minor, but did you consider adding dividers between the segments?

Yeah, I did consider this. I actually put it off and kind of forgot about it. The reason I originally didn't add it was because the logic was a bit strange before I was manipulating the view outside of the ItemCell. Now that we already have that in for the animation, I will go back and add this in as well.

@r-jarvis
Copy link
Collaborator Author

This is very minor, but did you consider adding dividers between the segments?

Finished implemntation.

@roanutil roanutil merged commit e4117a6 into main Nov 15, 2024
5 checks passed
@roanutil roanutil deleted the feature/segmented-picker-animation branch November 15, 2024 03:36
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.

2 participants