Skip to content

Conversation

@ze-flo
Copy link
Contributor

@ze-flo ze-flo commented Oct 24, 2024

Description

This PR updates various components in react-dropdowns to use transient props where appropriate. These changes are necessary in preparation for the upgrade to [email protected] to ensure we avoid DOM violation errors after the transition.

Checklist

  • 👌 design updates will be Garden Designer approved (add the designer as a reviewer)
  • 🌐 demo is up-to-date (npm start)
  • ⬅️ renders as expected with reversed (RTL) direction
  • ⚫ renders as expected in dark mode
  • 🤘 renders as expected with Bedrock CSS (?bedrock)
  • 💂‍♂️ includes new unit tests. Maintain existing coverage (always >= 96%)
  • ♿ tested for WCAG 2.1 AA accessibility compliance
  • 📝 tested in Chrome, Firefox, Safari, and Edge

@coveralls
Copy link

coveralls commented Oct 24, 2024

Coverage Status

coverage: 95.891%. remained the same
when pulling e5f669d on ze-flo/transient-props-dropdowns
into 5a6c53c on main.

@ze-flo
Copy link
Contributor Author

ze-flo commented Oct 24, 2024

This PR unintentionally fixes a misalignment of the MenuItem icon when isCompact is set to true.

See this comment for details.

@ze-flo ze-flo marked this pull request as ready for review October 24, 2024 00:48
@ze-flo ze-flo requested a review from a team as a code owner October 24, 2024 00:48
@ze-flo
Copy link
Contributor Author

ze-flo commented Oct 24, 2024

All tests have passed as expected after snapshot update ae9e4f2. 🙌

maxHeight,
focusInset,
validation,
...(getTriggerProps({
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
...(getTriggerProps({
const triggerProps = getTriggerProps({

I don't think the spread is needed any longer

@ze-flo ze-flo merged commit 0c856e8 into main Oct 24, 2024
8 checks passed
@ze-flo ze-flo deleted the ze-flo/transient-props-dropdowns branch October 24, 2024 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants