-
Notifications
You must be signed in to change notification settings - Fork 0
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: adds tooltip max width prop and removes events #285
Conversation
✅ Deploy Preview for pine-design-system ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -37,6 +37,7 @@ | |||
box-shadow: var(--box-shadow-default); | |||
color: var(--color-text-default); | |||
// TODO: need to use block / none but the tooltip content width and height are needed for calculations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we know what this TODO is for and are we able to clean this up while we are in here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@monicawheeler TBH, I have no idea but will check into it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe @QuintonJason could help -> 84f6259
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already pinged him!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@monicawheeler checked in with @QuintonJason and that comment ties into the universal approach of how tooltip, dropdown, popover will work. Quinton will be following up here as well as he begins work on the other mentioned components, so no action needed in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the follow up!
Description
This PR adds a
maxWidth
prop to thepds-tooltip
component, allowing users to set a maximum width for the tooltip content. It also sets the default to 352px as defined by the design team.In addition, as previously discussed by the team, events have been removed.
Fixes:
https://kajabi.atlassian.net/browse/DSS-1151
https://kajabi.atlassian.net/browse/DSS-1158
Type of change
Please delete options that are not relevant.
If your type of change is not present, add that option.
How Has This Been Tested?
Navigate to tooltip
Verify
maxWidth
is applied properly when added.Verify UI changes as expected
Verify documentation additions/updates are clear.
unit tests
e2e tests
accessibility tests
tested manually
other:
Test Configuration:
Checklist:
If not applicable, leave options unchecked.