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 cluster type tooltip message formatting #2864

Merged
merged 1 commit into from
Aug 6, 2024
Merged

Conversation

EMaksy
Copy link
Member

@EMaksy EMaksy commented Aug 6, 2024

Description

This little pr fixes the spacing problem on the cluster type tooltip message. In chrome and chromium no empty space was rendered. Firefox rendered the tooltip message correctly.

Played with different Whitespaces options in tailwindcss without success, for some reasons in chrome "whitespace-pre" class was not applied, wrapping it in a span applies the css rules in chrome and chromium

Correct rendering in Firefox:

image

Wrong rendering in Chrome and Chromium

Space between "migration" and "to Angi architecture"
image

Fixed rendering in Chrome and Chromium

image

How was this tested?

Tested on:

  • Google Chrome Version 125.0.6422.141 manually
  • Chromium Version 127.0.6533.88 manually
  • Mozilla Firefox 127.0.2 manually

@EMaksy EMaksy added the bug Something isn't working label Aug 6, 2024
@EMaksy EMaksy self-assigned this Aug 6, 2024
@EMaksy EMaksy requested review from arbulu89 and abravosuse August 6, 2024 14:09
@EMaksy
Copy link
Member Author

EMaksy commented Aug 6, 2024

An alternative solution would be also to change the position of empty space after "migration"

const CLASSIC_TOOLTIP_MESSAGE = (
  <>
    Classic architecture. Recommended{' '}
    <Link
      to={MIGRATION_URL}
      className="text-jungle-green-500 hover:opacity-75"
      target="_blank"
    >
      migration{' '}
    </Link>
    to Angi architecture
  </>
);

@EMaksy EMaksy marked this pull request as ready for review August 6, 2024 14:24
@EMaksy EMaksy requested a review from CDimonaco August 6, 2024 14:24
Copy link
Contributor

@abravosuse abravosuse left a comment

Choose a reason for hiding this comment

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

LGTM Eugen. Thanks!

Copy link
Contributor

@arbulu89 arbulu89 left a comment

Choose a reason for hiding this comment

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

I don't know why wrapping a span into other span changes things...
But hey, I'm not gonna spend one single minute fighting css war XD

@EMaksy EMaksy force-pushed the fix_chrome_formatting branch from 231d360 to d4a4a31 Compare August 6, 2024 16:21
@EMaksy EMaksy merged commit a742b1c into main Aug 6, 2024
26 checks passed
@EMaksy EMaksy deleted the fix_chrome_formatting branch August 6, 2024 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

3 participants