-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
UnitControl: preserve order of incoming units #62049
Conversation
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
Size Change: +73 B (0%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
nonNullValueProp, | ||
unitProp, | ||
unitsProp | ||
); | ||
// If allowedUnitValues is provided, we need to filter the list of units. | ||
if ( allowedUnitValues?.length ) { |
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.
So this is a huge hack and just here to illustrate the proposal.
The goal is for this component to honor the order of units. For example, if I wanted to have %
appear at the top of the unit list.
I'm just not sure whether to do it in the component or as part of useCustomUnits
.
#31822 (comment) suggests that useCustomUnits
might be the wrong place to do it.
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.
cc @ciampo for advice - non-urgent, when you get time 🙇🏻
…ds (eventually) to act in some way like `useCustomUnits` hook, but for now just filters the default units and preserves the order.
85c56d3
to
4b7ab27
Compare
Closing this as stale. I might come back to this idea one day. |
Sorry for the delay, I was on a parental leave and I'm still catching up with the notification queue. Happy to discuss the new approach if/when the time comes! |
What?
Testing passing an allowedUnitValues prop to UnitControl, which intends (eventually) to act in some way like
useCustomUnits
hook, but for now just filters the default units and preserves the order.See: #61928 (comment)
See also: #31822 (comment)
Why?
Consumers should be able to determine the order of the units in the control.
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast