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

Select list option groups #2111

Merged
merged 69 commits into from
Jun 4, 2024
Merged

Select list option groups #2111

merged 69 commits into from
Jun 4, 2024

Conversation

atmgrifter00
Copy link
Contributor

@atmgrifter00 atmgrifter00 commented May 15, 2024

Pull Request

🤨 Rationale

This PR only introduces groups to the Select. Providing this feature for the Combobox will be handled separately.

👩‍💻 Implementation

Introduces the ListOptionGroup component. This component required some APIs that manage the visibility of visual separators between groups and other visuals. This management occurs by the Select handling when the hidden or visibilityHidden state of a ListOptionGroup changes and then updating the necessary states on both the group that changed its hidden state and the adjacent groups if present. The ListOptionGroup manages its own visibilityHidden\hidden state based on the aggregate state of its contained ListOptions.

I've also changed the ListOption to assign itself to the option slot that now exists in both the ListOptionGroup template and all of the various dropdown components (i.e. Select, Combobox, and RichTextMentionListbox). This follows a similar pattern to that of the Tabs component and its related pieces.

Out of scope:

  • Explicit handling of groups within groups. It may be prudent to provide a more robust implementation that sets some validity state on the Select itself. For now, if a client provides a nimble-list-option-group within another nimble-list-option-group, nothing will get displayed in the dropdown, but no exception is thrown. The documentation currently states that this is unsupported.

🧪 Testing

Unit tests. Matrix tests.

✅ Checklist

  • I have updated the project documentation to reflect my changes or determined no changes are needed.

packages/nimble-components/src/list-option-group/index.ts Outdated Show resolved Hide resolved
packages/nimble-components/src/list-option-group/index.ts Outdated Show resolved Hide resolved
packages/nimble-components/src/list-option-group/styles.ts Outdated Show resolved Hide resolved
packages/nimble-components/src/select/index.ts Outdated Show resolved Hide resolved
packages/nimble-components/src/select/styles.ts Outdated Show resolved Hide resolved
packages/nimble-components/src/select/styles.ts Outdated Show resolved Hide resolved
packages/nimble-components/src/select/template.ts Outdated Show resolved Hide resolved
@atmgrifter00 atmgrifter00 linked an issue Jun 3, 2024 that may be closed by this pull request
@atmgrifter00 atmgrifter00 removed a link to an issue Jun 3, 2024
@rajsite
Copy link
Member

rajsite commented Jun 3, 2024

@atmgrifter00 I think the richtext changes are trivial and non-breaking so don't need to wait for @vivinkrishna-ni

@atmgrifter00 atmgrifter00 enabled auto-merge (squash) June 3, 2024 22:38
@atmgrifter00 atmgrifter00 merged commit 9ed4ff0 into main Jun 4, 2024
13 checks passed
@atmgrifter00 atmgrifter00 deleted the list-option-groups branch June 4, 2024 01:51
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.

4 participants