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

feat(menuitem): add determinate/indeterminate variant to the MenuItem #602

Conversation

masoudmanson
Copy link
Contributor

Summary

MenuItem
Github issue: #601

As part of implementing the new Multi-column dropdown, we need to temporarily introduce a determinate/indeterminate variant for the MenuItem component. This change is temporary as we're planning to use the InputCheckbox component inside the MenuItem component to handle checkbox states instead of directly managing them within the MenuItem component.

Checklist

  • Default Story in Storybook
  • LivePreview Story in Storybook
  • Test Story in Storybook
  • Tests written
  • Variables from defaultTheme.ts used wherever possible
  • If updating an existing component, depreciate flag has been used where necessary
  • Chromatic build verified by @chanzuckerberg/sds-design

Copy link

@clarsen-czi clarsen-czi left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @masoudmanson !!

@masoudmanson masoudmanson merged commit 7c7e3b3 into main Sep 14, 2023
7 checks passed
@masoudmanson masoudmanson deleted the 601-add-determinateindeterminate-variant-to-the-menuitem-component branch September 14, 2023 18:41
@@ -110,9 +110,9 @@
"build-storybook": "storybook build -o docs-build",
"test-storybook": "test-storybook",
"storybook:axe": "yarn build-storybook && yarn storybook:axeOnly",
"storybook:axeOnly": "axe-storybook --build-dir docs-build",
"storybook:axeOnly": "axe-storybook --build-dir docs-build --timeout 10000",
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for adding this, @masoudmanson !

We can probably remove the following config in test files now we set the flag globally!

axe: {
  timeout: 10 * 1000,
},

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, you're right.
We don't even need a global timeout config. I'll revert this change and set a timeout for that specific component. 😯😥

#611

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Determinate/Indeterminate variant to the MenuItem component
3 participants