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
[Controls] Field first control creation #131461
[Controls] Field first control creation #131461
Changes from 10 commits
45a4397
afcac67
e032a2f
2aaeba9
57d1280
5433c55
839a1f2
0156e9e
aaf7ac4
2e7d2ef
c0feed0
e95afac
c200b09
f306785
0334364
f8b6027
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.
I wonder if we should use the friendly name of the selected Control Type here, so it would read
Options List settings
orRange slider settings
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.
Great point! @andreadelrio what are your thoughts?
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.
I'm thinking if we need to show that label at all?
Label
andWidth
are settings too right so no need to have a separate section for (other) settings? or are you trying to keep the toggle for width underMinimum width
and so you need a heading for the rest of the settings?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.
@andreadelrio Yeah, it seemed to me like grouping the type-specific control settings with the
Minimum width
settings was confusing - so either the type-specific settings should go above theMinimum width
(which would cause shifting when switching betweenOptions list
andRange slider
fields) or I think these type-specific settings should have their own label.What do you think, though?
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.
I think we could either group them under something like Additional settings? or give each toggle its own title so something like:
Allow multiple selections in dropdown
Run past timeout
What do you think @KOTungseth ?
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.
I like
Additional settings
but definitely interested in hearing @KOTungseth's thoughts! :)