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

Tooltip indicators don't play well with code-formatted text #1231

Open
damithc opened this issue Jun 5, 2020 · 4 comments
Open

Tooltip indicators don't play well with code-formatted text #1231

damithc opened this issue Jun 5, 2020 · 4 comments
Labels
p.Medium s.UnderDiscussion The team will evaluate this issue to decide whether it is worth adding

Comments

@damithc
Copy link
Contributor

damithc commented Jun 5, 2020

v2.14.1

image

Problem: the underline that indicates the tooltip doesn't show under text that are code-formatted

@ang-zeyu
Copy link
Contributor

ang-zeyu commented Jun 7, 2020

This is one of the drawbacks of using border to underlining things I suppose. We had issues with text-decoration: underline and icons in #995 though.

Untitled

^ this is done by applying a descendant selector .trigger > code, but the issue occured because code elements have a background color which blocks the border behind it. Not too sure if there's a more generalizable solution. Could get some input from @acjh @yamgent

@ang-zeyu
Copy link
Contributor

@damithc #1350 fixes this (as well as popover / tooltip positioning), but introduces the underline for everything. (note that its visible on buttons now as well)

Untitled

the relavant style in #1133 was likely intentionally removed by @hcwong to avoid this

also note how the code element now increases the line height, this may not be desired

Untitled

what do you think? @hcwong @damithc

@damithc
Copy link
Contributor Author

damithc commented Sep 12, 2020

what do you think? @hcwong @damithc

Both side effects are, as you noted, undesirable. Ideally, should avoid. If cannot, may be keep the current status quo for the time being?

@ang-zeyu
Copy link
Contributor

ang-zeyu commented Sep 12, 2020

current status quo for the time being?

I don't think there's a clean solution to this, best to decide which tradeoffs to accept =X

I really think the positioning should be fixed though, its not so pronounced in the case of buttons because its not very large;
if you add it to something like an image then its very visible

also note the line height disruption is only applicable when all the elements you place in the popover/trigger/tooltip are inline; If they are inline-block (e.g. buttons) its going to be disrupted eitherway

assuming display: inline-block which fixes the positioning, some possible approaches:

  1. revert to text-decoration
    • underline support for things without text (e.g. fa icons) breaks entirely
  2. keep border approach to underlining
    • underline appears for everything
    • line height is disrupted even when unnecessary because of the border
  3. style detection, use text-decoration by default, border for elements without text
    • inconsistent styling between border and text-decoration approaches
    • small performance hit
  4. keep border approach, provide a user option to use text-decoration instead of border
    • inconsistent styling between border and text-decoration approaches
    • extra work on user side

assuming no display: inline-block:

  • positioning breaks in all cases when non-inline elements are involved (e.g. buttons, images)
  1. revert to text-decoration
  2. keep border approach to underlining
    • this issue stays unresolved
  3. style detection, use text-decoration by default, border for elements without text
    • inconsistent styling between border and text-decoration approaches
    • small performance hit
  4. keep border approach, provide a user option to use text-decoration instead of border
    • inconsistent styling between border and text-decoration approaches
    • extra work on user side

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p.Medium s.UnderDiscussion The team will evaluate this issue to decide whether it is worth adding
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants