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(16575): modify useIsTruncated hook in ListBoxMenuItem #16576

Conversation

ahmedsemih
Copy link
Contributor

@ahmedsemih ahmedsemih commented May 26, 2024

Reference #16575

When the dropdown item is selected, in the control in useIsTruncated hook, element.lastElementChild?.lastElementChild always be the path of the checkmark svg. OffsetWidth of this path was undefined and scrollWidth was 0. Because of that useIsTruncated returns false and the tooltip was not working. I modified the inside of the hook. it always gets the offsetWidth and scrollWidth of the element so it works fine.

Changelog

New

  • N/A

Changed

  • Modified useIsTruncated hook in ListBoxMenuItem component to always use the element's offsetWidth and scrollWidth, fixing the issue with the tooltip not displaying.

Removed

  • Removed the nested element control from the useIsTruncated hook that references the path of the checkmark svg.

Testing / Reviewing

The full text of the dropdown item now appears in the tooltip on hover, whether the item is selected or not. You can see the issue before it was fixed in the issue closed by this commit.

@ahmedsemih ahmedsemih requested a review from a team as a code owner May 26, 2024 07:45
Copy link

netlify bot commented May 26, 2024

Deploy Preview for v11-carbon-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 038bc49
🔍 Latest deploy log https://app.netlify.com/sites/v11-carbon-react/deploys/6652e89eafbd210008e3f887
😎 Deploy Preview https://deploy-preview-16576--v11-carbon-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 configuration.

Copy link

netlify bot commented May 26, 2024

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 038bc49
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/6652e89ee551d90008634c58
😎 Deploy Preview https://deploy-preview-16576--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 configuration.

Copy link
Contributor

@Gururajj77 Gururajj77 left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, looks good!

@guidari
Copy link
Contributor

guidari commented May 28, 2024

Awesome! Thanks for the fix!

I'll keep the issue open because is was reported on the v10 branch, but I'll reference this PR if someone wants to backport the fix.

@alisonjoseph alisonjoseph added this pull request to the merge queue May 29, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 29, 2024
@alisonjoseph alisonjoseph added this pull request to the merge queue Jun 3, 2024
Merged via the queue into carbon-design-system:main with commit 364ffc8 Jun 3, 2024
21 checks passed
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