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 tooltip-position inside scaleModal #1874

Closed
wants to merge 3 commits into from
Closed

Conversation

niclas18
Copy link
Contributor

As described in #1742 there is a problem with the position of tooltips inside modals.
The used package @floating-ui/dom have a problem with the shadow-roots there.

They have a fix in there documentation for this type of problems.
I added this and it fixed the problem for us.

@niclas18 niclas18 requested review from acstll and nowseemee as code owners May 23, 2023 13:29
@netlify
Copy link

netlify bot commented May 23, 2023

Deploy Preview for marvelous-moxie-a6e2fe ready!

Name Link
🔨 Latest commit ada2fa9
🔍 Latest deploy log https://app.netlify.com/sites/marvelous-moxie-a6e2fe/deploys/646ddf514a3472000720cde9
😎 Deploy Preview https://deploy-preview-1874--marvelous-moxie-a6e2fe.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.

@acstll
Copy link
Collaborator

acstll commented May 23, 2023

Hey @niclas18 this is brilliant, thank you.

In order to make CI pass, would you be so kind and please do the following:

  • run yarn format inside packages/components
  • commit yarn.lock in the root

@niclas18
Copy link
Contributor Author

Hey @niclas18 this is brilliant, thank you.

In order to make CI pass, would you be so kind and please do the following:

  • run yarn format inside packages/components
  • commit yarn.lock in the root

Hi @acstll , thanks for your comment.
I have completed the requested steps. In addition, I discovered another problem, which is why I adjusted the CSS a bit.

If the trigger of the tooltip is relatively at the edge of an element and the associated message is very long, then the arrow is no longer over the element. This can also be reproduced in the storybook.

This also happens in the modal. By changing the CSS in the PR and changing the CSS of the corresponding scale modal:

scale-modal::part(window) {
    position: static;
}

it now works reliably.

Maybe you could see if it is necessary to use "position: relative" in ScaleModal. If not and you change it globally to "position: static", one wouldn't need the above code.

beforehand:
image

afterward:
image

@andoo
Copy link

andoo commented Oct 30, 2023

Hello, is there any chance to get this merged? Scale tooltips inside modal dialogs have been a problem in our application for a long time. And I would like to wait for this possible fix before deciding to create our own tooltip component.

@felix-ico
Copy link
Collaborator

felix-ico commented Nov 15, 2023

hi there thanks a lot for the PR and sorry for the delay, we duplicated this PRs changes here #2206 to fix the CI jobs - we should merge and publish this with the next release.

@felix-ico
Copy link
Collaborator

closing this as the dupe pr has just been merged, thanks again for this one

@felix-ico felix-ico closed this Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants