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

[web] Tidy reused Menu components #48904

Merged
merged 2 commits into from
Nov 18, 2024
Merged

[web] Tidy reused Menu components #48904

merged 2 commits into from
Nov 18, 2024

Conversation

kiosion
Copy link
Contributor

@kiosion kiosion commented Nov 13, 2024

Add generic MultiselectMenu, ViewModeSwitch, and SortMenu components, replace usages in UnifiedResources.

Resolves #5457.

@kiosion kiosion added the no-changelog Indicates that a PR does not require a changelog entry label Nov 13, 2024
@kiosion kiosion force-pushed the maxim/tidy-menus branch 4 times, most recently from 48a53f3 to 491b36d Compare November 13, 2024 17:36
Copy link
Contributor

@rudream rudream left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with comment

web/packages/shared/components/Controls/ViewModeSwitch.tsx Outdated Show resolved Hide resolved
@@ -164,8 +164,7 @@ export const MultiselectMenu = <T extends string>({
<>
<CheckboxInput
type="checkbox"
// @ts-expect-error assigning ReactNode to checkbox name field
name={opt.label}
name={opt.value}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah, opt.value is much better and actually aligns with what this attribute is for – it completely escaped my head when I was writing the original comment.

@kiosion kiosion force-pushed the maxim/tidy-menus branch 2 times, most recently from da953ff to c34f66d Compare November 18, 2024 17:41
* Add generic MultiselectMenu, ViewModeSwitch, SortMenu components
@kiosion kiosion enabled auto-merge November 18, 2024 17:53
@kiosion kiosion added this pull request to the merge queue Nov 18, 2024
Merged via the queue into master with commit 192e8eb Nov 18, 2024
38 checks passed
@kiosion kiosion deleted the maxim/tidy-menus branch November 18, 2024 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Indicates that a PR does not require a changelog entry size/md ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants