Add Options Group #990
Add Options Group #990
Conversation
🦋 Changeset detectedLatest commit: 8ded269 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
65c9ee5 to
a7fc13c
Compare
| ), | ||
| ...this.querySelectorAll<Menu>( | ||
| // The "content" slot and Options Group case. | ||
| ':scope > glide-core-options > glide-core-options-group > glide-core-option > [slot="content"] > glide-core-menu', |
src/option.styles.ts
Outdated
| } | ||
| .container { | ||
| block-size: 1.75rem; |
There was a problem hiding this comment.
Now 28 pixels tall instead of 27. One pixel difference. So I thought I'd skip a changeset for it. Eh?
There was a problem hiding this comment.
I'm on the fence. It is a change that consumers may notice if they have visual tests, similar to how ours detected the diff. So part of me thinks it may be wise to call it out explicitly for visibility.
But I can also communicate this outwardly to folks if we don't feel it's worth one.
There was a problem hiding this comment.
The visual tests thing is a good point. I'll add a changeset. Thanks.
| /** | ||
| * @attr {string} label | ||
| * @attr {boolean} [hide-label=false] | ||
| * @attr {'group'|'optgroup'} [role='group'] |
There was a problem hiding this comment.
optgroup for Select. Select will look for Options Groups in its default slot and set role to "optgroup" for consumers.
00d409a to
d58ec49
Compare
d58ec49 to
6b55f6a
Compare
There was a problem hiding this comment.
Looks like a minor issue when an Option has a description. Can see the issue here and here with an Option Group + Option description.
To reproduce:
- Go to the Menu story
- Add a
descriptionto Option
src/option.styles.ts
Outdated
| } | ||
| .container { | ||
| block-size: 1.75rem; |
There was a problem hiding this comment.
I'm on the fence. It is a change that consumers may notice if they have visual tests, similar to how ours detected the diff. So part of me thinks it may be wise to call it out explicitly for visibility.
But I can also communicate this outwardly to folks if we don't feel it's worth one.
Derp. Thank you. That's what I get for a making that last minute change. I'll push a fix. |
1ece024 to
8ded269
Compare
|
🚀 Description
Adds an Options Group component for use in Menu and Select.
📋 Checklist
🔬 Testing