Skip to content

fix: tooltip rendering #122

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.

Sign up for GitHub

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

Open
fsimba-dev wants to merge 15 commits into
base: main
Choose a base branch
from
Open

fix: tooltip rendering #122

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
e6ffee8
fix: tooltip rendering
fsimba-dev Aug 18, 2025
692fe33
fix issues related to commit review
fsimba-dev Aug 29, 2025
93aca9b
Merge branch 'main' into 108-fix-Tooltip-rendering
AMorgaut Sep 1, 2025
fb68549
build(deps): bump vue in the production-dependencies group
dependabot[bot] Sep 1, 2025
8b5c250
build(deps-dev): bump the development-dependencies group with 3 updates
dependabot[bot] Sep 2, 2025
07998ad
build(sonar-services): add project branch test
AMorgaut Sep 7, 2025
ca35475
feat(sonar-services): add pull request list
AMorgaut Sep 7, 2025
033d4b3
feat(sonar-services): add rule list and detail
AMorgaut Sep 7, 2025
089bdd3
feat(core-services): add priority rule + tests
AMorgaut Sep 7, 2025
d232b37
fix(core-services): fix lint issue
AMorgaut Sep 7, 2025
80d7765
fix(storybook)
AMorgaut Sep 9, 2025
2456631
feat(vue-ui): add DashboardPageTemplate
jasdubourg Aug 4, 2025
4b4067f
build(vue-ui): dynamic data for score block and tags with stories
jasdubourg Aug 21, 2025
5b7bb53
feat(core-services): add the getScoreTexts() method
jasdubourg Aug 22, 2025
10f9fc1
build(vue-ui): correction for input data dashboard
jasdubourg Aug 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions shared/vue-ui/src/molecules/BulletCriticity/BulletCriticity.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,17 @@ li {
font-size: 12px;
margin-bottom: 17px;
}

li strong {
margin-top: auto;
margin-left: auto;
margin-right: 10px;
}

li svg{
margin-top: auto;
margin-bottom: 2px;
}
.clickable {
cursor: pointer;
}
Expand Down
28 changes: 14 additions & 14 deletions shared/vue-ui/src/molecules/TooltipBox/TooltipBox.stories.js
View file
Open in desktop
Comment thread
AMorgaut marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,6 @@ export default {
anchor: { control: { type: 'boolean' }, required: false, default: false },
default: { control: { type: 'text' }, required: true },
},
template: `
<p>
The
<abbr>
HTML
<tooltip-box v-bind="args">
</tooltip-box>
</abbr>
abbr element.
</p>
`,
}

export const WithAnchor = {

@AMorgaut AMorgaut Aug 18, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

issue: This story was meant to use the HTML template you removed from default. The "info" icon was expected to appear at the right of "HTML" in the sentence "The HTML abbr element."

@AMorgaut AMorgaut Aug 20, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If I try to do

const Template = (args) => ({
  components: { TooltipBox },
  setup() {
    return { args }
  },
  template: `
      <p>
        The <abbr>
          HTML
          <TooltipBox v-bind="args">{{ args.default }</TooltipBox>
        </abbr>
        abbr element.
      </p>
  `,
});

export const WithAnchor = Template.bind({});
WithAnchor.args = {
  anchor: true,
  default: "Hyper Text Markup Language"
}

I see the anchor is clearly not aside its target
image

@fsimba-dev fsimba-dev Aug 28, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It is not easy to simulate the behavior of this component with the storybook (render issue).
But if you try to use it in real condition it works properly

@AMorgaut AMorgaut Sep 1, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

To me there must be an issue if we have this behavior

But we may handle this one in a separated ticket if we don't meet it in our current usages

Expand All @@ -29,8 +18,19 @@ export const WithAnchor = {
}
}

export const UseParentOnlyAsAnchor = {
args: {
default: "Hyper Text Markup Language"
const Template = (args) => ({

@AMorgaut AMorgaut Aug 18, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

niptick: this template is not an exported story so it could be less confusing to define it before the WithAnchor story (Moreover, the WithAnchor story should use it too...)

components: { TooltipBox },
setup() {
return { args }
},
template: `

@AMorgaut AMorgaut Aug 29, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

issue: The template still doesn't use the expected right HTML exemple

It should be:

{
  template: `
      <p>
        The
        <abbr>
          HTML
          <tooltip-box v-bind="args">{{ args.default }}</tooltip-box>
        </abbr>
        abbr element.
      </p>`
}

The current example is wrong because it doesn't highlight the difference between

  • the text holding the tooltip (here "HTML") which will react on mouse over,
  • and the other text on which there will be no interaction (here "The" and "abbr element.")

<p>Mouse hover this text to display the tooltip content.</p>
<tooltip-box v-bind="args">{{ args.default }}</tooltip-box>
`,
});

export const UseParentOnlyAsAnchor = Template.bind({});

@AMorgaut AMorgaut Aug 20, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

niptick: when we use the right example template without the anchor, there is no visual indication showing the "HTML" text in the <abbr /> element has an associated tooltip. The parent element of the tooltip component should probably have something like a dashed underline decoration which is a more commonly applied style to highlight content with alternate text such as tooltips.

UseParentOnlyAsAnchor.args = {
default: "Hyper Text Markup Languag",
anchor: false,
}
12 changes: 6 additions & 6 deletions shared/vue-ui/src/molecules/TooltipBox/TooltipBox.vue
View file
Open in desktop

@AMorgaut AMorgaut Aug 20, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

issue: the tooltip content is not displayed aside the anchor

image

@fsimba-dev fsimba-dev Aug 28, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Same problem as above ... We got some render issues due to the storybook ...
It should work properly in real condition (example TagCard)

@AMorgaut AMorgaut Sep 1, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As I said we may handle it in a separated ticket
Moreover I don't think it is relevant to a use case with the anchor without associated text to attach it to

It is ok in the TagCard but probably because of a specific context

@AMorgaut AMorgaut Sep 1, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

todo: it would be nice to add a TooltipBox.spec.js unit test file as the one we have for the AbcdeScore component. It would also help to pass the Sonar Gateway

Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ defineProps({
});

</script>

<template>
<div

@AMorgaut AMorgaut Aug 20, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

issue: the accessibility tests show a color-contrast issue. Moreover, it mentions that bit is difficult for a screen reader to know if the "i" text has some meaning

image

@fsimba-dev fsimba-dev Aug 28, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I tried to adjust the color a better contrast ratio but I still got this warning (even if i put black and white which meets WCAG AAA standard).

I think the issue is coming from the text content which is too short.

@AMorgaut AMorgaut Sep 1, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

thank you for trying it

A solution might be to consider that "i" is an abreviation for "info and then to replace

<div class="anchor">i</div>

by

<abbr title="information" class="anchor">i</abbr>

@fsimba-dev fsimba-dev Sep 10, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Still got the same issue (contrast color warning) when using this solution ...

v-if="anchor"
Expand Down Expand Up @@ -37,11 +38,12 @@ defineProps({
font-size: smaller;
float: right;
margin-left: 5px;

@AMorgaut AMorgaut Aug 20, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

issue: the "i" in the anchor looks to not be well verticaly aligned in the blue disk, also the text is selectable and as not the right pointer cursor when we mouse the mouse on it

@fsimba-dev fsimba-dev Sep 10, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

fixed

margin-top: 5px;
}

div[role=tooltip] {

--_p-inline: 1.5ch;
--_p-inline: 2.5ch;
--_p-block: .75ch;
--_triangle-size: 7px;
--_shadow-alpha: 50%;
Expand All @@ -62,26 +64,24 @@ div[role=tooltip] {

inline-size: max-content;

max-inline-size: 25ch;
max-inline-size: 50ch;

text-align: start;

font-size: 1rem;

font-weight: normal;
line-height: normal;
line-height: initial;

padding: var(--_p-block) var(--_p-inline);
margin: 0;

border: 1px solid;
border-radius: 5px;

/* Use System Colors https://developer.mozilla.org/en-US/docs/Web/CSS/system-color */
color: CanvasText;
background-color: Canvas;

border: 1px solid

}

/* create a stacking context for elements with tooltips */
Expand Down
11 changes: 3 additions & 8 deletions shared/vue-ui/src/organisms/ScoreBlock/ScoreBlock.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,9 @@ const projectLink = computed(() => {
<div class="tooltip-calc">
Comment thread
AMorgaut marked this conversation as resolved.
how my score is calculated ?
<tooltip-box>
<p>
<strong>{{ labelLong }} </strong><br>
{{ text }}
</p>
<br>
<p>
<strong>Identified issues in your app:</strong>
</p>
<strong>{{ labelLong }}</strong>
<br><br>
<strong>Identified issues in your app:</strong>
<rule-criticities

@AMorgaut AMorgaut Aug 20, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

issue: The is no padding on the right.
The 0 values are stick to the border
image

@fsimba-dev fsimba-dev Sep 10, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

fixed

:minor-severities="minorSeverities"
:major-severities="majorSeverities"
Expand Down
18 changes: 7 additions & 11 deletions shared/vue-ui/src/organisms/TagCard/TagCard.vue
View file
Open in desktop

@AMorgaut AMorgaut Aug 20, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

issue: the anchor should be upper than its target text. we can also see on this capture the positionning issue of the "i" in the the blue disc

image

You can refer to the visual reference there in the TagCard feature request: #91

@AMorgaut AMorgaut Sep 1, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

is this issue fixed?

@fsimba-dev fsimba-dev Sep 10, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes the position of the letter 'i' was centered ...
For the tooltip position we can pass it through the component props (top, bottom, left, ...) ...
The default position is bottom

Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,13 @@ const projectLink = computed(() => {
<template>
<div class="tag-card">
<div class="head">
<div>
<div class="title">
<strong>{{ props.metricTag }}</strong>
</div>
<div>
<tooltip-box anchor>
Affected rules correspond to the number of rules associated with
errors identified by SonarQube, among the Greensight rules that
impact this component
</tooltip-box>
</div>
<div class="title">
<strong>{{ props.metricTag }}</strong>
<tooltip-box anchor>
Affected rules correspond to the number of rules associated with
errors identified by SonarQube, among the Greensight rules that
impact this component
</tooltip-box>
</div>
<a
v-if="!noIssues"
Expand Down