Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[Components] Add SegmentedControl #405
[Components] Add SegmentedControl #405
Changes from 7 commits
b3443d3
073eac8
1b2ef12
555fa60
3cb4c78
f6b66ae
2fdf3ca
0cf1851
a09919b
762bc2f
54aff38
8bfbb8a
ef5439a
af00a01
cc99778
cdbd2ac
59c38d7
edd2717
c342101
959d467
6f55c81
dbf0ac7
efd5c99
aaab47f
9ce66a1
2add20e
b4fbec0
4f75e46
49de0c3
b6005d7
57f6de2
41d7b89
f611f53
c23a2d9
9809faf
87e96b7
23b393f
d5eb34b
1164df8
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to consolidate
Indicator
within thesegmentItems
map function to prevent adding an additional button to the DOM? It looks like there's always one more than there existssegmentItems
:Also, since Indicator precedes all the segment items, the current keyboard navigation behave is to immediately tab to the currently active segment. Is this intended? E.g., if Option 2 of 3 options is active, tabbing will go to 2, then to 1, then to 3. My expectation is that it would go in order.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ben will probably be able to provide more guidance re: styling for different states, but there's a new
focus
variable you can use here to start playing around with how the component would work with only keyboard controls:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah yeah, thanks for calling that out @michaeljaltamirano but we should apply the new focus styling to this: unsure on how it will react between the two options
focus
andfocusInner
but would it be possible to get a screenshot of both with the two scenarios below. These are how it looks with designs but unsure how it will with live codeThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@heyamykate I ended up getting out of being underwater this cycle--can you re-tag me for review once the keyboard controls are in place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@heyamykate It looks like there's some CSS finessing to match the above:
Active item has focus state:
Inactive item has focus state:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@heyamykate The focused state looks perfect for the active item.
However, if an inactive item adjacent still has a small piece cutoff:
If you add a z-index, it should look like below: