Skip to content

fix: tooltip rendering#122

Open
fsimba-dev wants to merge 15 commits into
mainfrom
108-fix-Tooltip-rendering
Open

fix: tooltip rendering#122
fsimba-dev wants to merge 15 commits into
mainfrom
108-fix-Tooltip-rendering

Conversation

@fsimba-dev

Copy link
Copy Markdown

No description provided.

@fsimba-dev
fsimba-dev requested a review from AMorgaut August 18, 2025 07:36
export const UseParentOnlyAsAnchor = {
args: {
default: "Hyper Text Markup Language"
const Template = (args) => ({

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...)

`,
}

export const WithAnchor = {

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

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

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

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

Comment thread shared/vue-ui/src/molecules/TooltipBox/TooltipBox.stories.js

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

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

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

@@ -37,11 +38,12 @@ defineProps({
font-size: smaller;
float: right;
margin-left: 5px;

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

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

</script>

<template>
<div

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

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.

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>

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 ...

`,
});

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

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.

<strong>{{ labelLong }}</strong>
<br><br>
<strong>Identified issues in your app:</strong>
<rule-criticities

@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 is no padding on the right.
The 0 values are stick to the border
image

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

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

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?

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

Comment thread shared/vue-ui/src/organisms/ScoreBlock/ScoreBlock.vue
.tooltip-calc {
margin-top: 20px;
text-decoration: underline;
text-decoration: underline dashed 1px;

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 text-decoration style should be moved at the Tooltip component level. I guess it should be in the CSS rule :has(> div[role=tooltip])

@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.

not easy to apply style to component caller (here the <div> in the score block)...
That's why I putted the in the caller component( score block)

text-decoration: underline;
text-decoration: underline dashed 1px;
display: inline-block;
cursor: help;

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 cursor style should be moved at the Tooltip component level. I guess it should be in the CSS rule :has(> div[role=tooltip])

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.

not easy to apply style to component caller (here the <div> in the score block)...
Thats why I putted the in the caller component( score block)

},
template: `
<p>Mouse hover this text to display the tooltip content.</p>
<p style='text-decoration: underline dashed 1px'>Mouse hover this text to display the tooltip content.</p>

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 style should not be in the story, it should be implemented by default by the tooltip component.

I guess it should be in the CSS rule :has(> div[role=tooltip]) and the div[role=tooltip] rule should clean it so the tooltip content is not underlined too

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

@AMorgaut AMorgaut Aug 29, 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 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.")

`,
});

export const WithAnchor = {

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 should use the same template

</script>

<template>
<div

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>

`,
}

export const WithAnchor = {

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

@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.

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

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

@sonarqubecloud

sonarqubecloud Bot commented Sep 1, 2025

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
E Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

dependabot Bot and others added 12 commits September 10, 2025 10:27
Bumps the production-dependencies group with 1 update: [vue](https://github.com/vuejs/core).


Updates `vue` from 3.5.18 to 3.5.20
- [Release notes](https://github.com/vuejs/core/releases)
- [Changelog](https://github.com/vuejs/core/blob/main/CHANGELOG.md)
- [Commits](vuejs/core@v3.5.18...v3.5.20)

---
updated-dependencies:
- dependency-name: vue
  dependency-version: 3.5.20
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: production-dependencies
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps the development-dependencies group with 3 updates: [vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite), [jest](https://github.com/jestjs/jest/tree/HEAD/packages/jest) and [msw](https://github.com/mswjs/msw).


Updates `vite` from 7.1.3 to 7.1.4
- [Release notes](https://github.com/vitejs/vite/releases)
- [Changelog](https://github.com/vitejs/vite/blob/main/packages/vite/CHANGELOG.md)
- [Commits](https://github.com/vitejs/vite/commits/v7.1.4/packages/vite)

Updates `jest` from 30.1.1 to 30.1.2
- [Release notes](https://github.com/jestjs/jest/releases)
- [Changelog](https://github.com/jestjs/jest/blob/main/CHANGELOG.md)
- [Commits](https://github.com/jestjs/jest/commits/HEAD/packages/jest)

Updates `msw` from 2.10.5 to 2.11.1
- [Release notes](https://github.com/mswjs/msw/releases)
- [Changelog](https://github.com/mswjs/msw/blob/main/CHANGELOG.md)
- [Commits](mswjs/msw@v2.10.5...v2.11.1)

---
updated-dependencies:
- dependency-name: vite
  dependency-version: 7.1.4
  dependency-type: direct:development
  update-type: version-update:semver-patch
  dependency-group: development-dependencies
- dependency-name: jest
  dependency-version: 30.1.2
  dependency-type: direct:development
  update-type: version-update:semver-patch
  dependency-group: development-dependencies
- dependency-name: msw
  dependency-version: 2.11.1
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: development-dependencies
...

Signed-off-by: dependabot[bot] <support@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🎨 design system javascript Pull requests that update javascript code

Development

Successfully merging this pull request may close these issues.

3 participants