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

fix(ListBoxMenuItem): ensure border-subtle renders correct value #13879

Merged

Conversation

tw15egan
Copy link
Member

@tw15egan tw15egan commented May 25, 2023

@aagonzales mention on Slack that the Dropdown / Combobox / Multiselect $border-subtle values were off. It's because the menus themselves render one level higher on the layer scale (01) , but border-subtle was still calculating to its base level (00). This change scopes all ListBoxMenuItem styles to their appropriate value, based on the presence of cds--layer-two or cds--layer-three, and defaults to border-subtle-01 if not. I searched for all $border-subtle instances and added a block for each of the two layers, and cleaned up some styles issues that were present with the change in color of these new borders.

Changelog

Changed

  • Adjusted instances of $border-subtle to reference the layer directly i.e $border-subtle-01

Testing / Reviewing

Check out the Dropdown, Combobox, Multiselect and other withLayer stories that utilize border-subtle and ensure values remain correct. THis mainly affects the menu items that appear when the menu is open, and when hovering them. This should only affect components that use ListBoxMenuItem.

@tw15egan tw15egan force-pushed the border-subtle-layer-dropdown branch from 8b94e40 to d4d16fb Compare May 25, 2023 20:48
@tw15egan tw15egan requested a review from tay1orjones May 25, 2023 20:49
@netlify
Copy link

netlify bot commented May 25, 2023

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 139979b
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/646fc8f6b5b9600008c24015
😎 Deploy Preview https://deploy-preview-13879--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented May 25, 2023

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 6ed82a5
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/6492f0dd4bd2f400088c7cca
😎 Deploy Preview https://deploy-preview-13879--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@tw15egan tw15egan force-pushed the border-subtle-layer-dropdown branch from 50b5aab to e8b7f80 Compare June 6, 2023 14:50
@netlify
Copy link

netlify bot commented Jun 6, 2023

Deploy Preview for carbon-components-react ready!

Name Link
🔨 Latest commit e8b7f80
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/647f47b15518090008cd5cbe
😎 Deploy Preview https://deploy-preview-13879--carbon-components-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Jun 6, 2023

Deploy Preview for carbon-components-react ready!

Name Link
🔨 Latest commit 6ed82a5
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/6492f0dd0df2180008bf1a21
😎 Deploy Preview https://deploy-preview-13879--carbon-components-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@tw15egan tw15egan force-pushed the border-subtle-layer-dropdown branch from b7d5c35 to d7814f0 Compare June 16, 2023 16:48
@tw15egan tw15egan marked this pull request as ready for review June 16, 2023 17:32
@tw15egan tw15egan requested a review from a team as a code owner June 16, 2023 17:32
Copy link
Member

@aagonzales aagonzales left a comment

Choose a reason for hiding this comment

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

Yes! That is rendering correctly now 🎉

Copy link
Contributor

@guidari guidari left a comment

Choose a reason for hiding this comment

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

LGTM!

@tw15egan
Copy link
Member Author

Looks like there is still an issue with Dropdown on Modal, just need to add another override for that use-case as well

Copy link
Member

@tay1orjones tay1orjones left a comment

Choose a reason for hiding this comment

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

Looks good once the modal override is added 👍

@tw15egan tw15egan force-pushed the border-subtle-layer-dropdown branch from d7814f0 to 6ed82a5 Compare June 21, 2023 12:45
@kodiakhq kodiakhq bot merged commit 99ffdd1 into carbon-design-system:main Jun 21, 2023
15 checks passed
@carbon-bot
Copy link
Contributor

Hey there! v11.32.0 was just released that references this issue/PR.

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

Successfully merging this pull request may close these issues.

5 participants