Skip to content

feat(web-components): add slot for an icon in ic-tooltip#4260

Open
GCHQDeveloper416 wants to merge 3 commits intodevelopfrom
734-tooltip-icon
Open

feat(web-components): add slot for an icon in ic-tooltip#4260
GCHQDeveloper416 wants to merge 3 commits intodevelopfrom
734-tooltip-icon

Conversation

@GCHQDeveloper416
Copy link
Contributor

@GCHQDeveloper416 GCHQDeveloper416 commented Feb 11, 2026 •
edited
Loading

Summary of the changes

Add slot for icon within tooltip. Tooltip icon stays on left and aligned with first line of text. Alt-text prop enables text to be added for the icons for screenreaders if alt text is given.
Capture

Related issue

#734

Checklist

General

  • Changes to docs package checked and committed.
  • All acceptance criteria reviewed and met.

Testing

  • Relevant unit tests and visual regression tests added.
  • Visual testing against Figma component specification completed.
  • Playground stories in React Storybook up to date, with any prop changes and additions addressed.
  • Compare performance of modified components against develop using Performance addon in React Storybook.

Accessibility

  • Accessibility Insights FastPass performed.
  • A11y unit test added and yields no issues.
  • A11y plug-in on Storybook yields no issues.
  • Manual screen reader testing performed using NVDA and VoiceOver.
  • Manual keyboard testing for keyboard controls and logical focus order.
  • Correct roles used and ARIA attributes used correctly where required.
  • Logical heading structure is maintained, and the HTML elements used for headings can be changed to fit within the wider page structure.

Resize/zoom behaviour

  • Page can be zoomed to 400% with no loss of content.
  • Screen magnifier used with no issues.
  • Text resized to 200% with no loss of content.
  • Text spacing increased as per the WCAG 1.4.12 success criterion with no loss of content.

System modes

  • Browser setting 'prefers reduced motion' tested. No animations or motion visible whilst this setting is on.
  • Windows High Contrast mode tested with no loss of content.
  • System light and dark mode tested with no loss of content.
  • Browser support tested (Chrome, Safari, Firefox and Edge).

Testing content extremes

  • Min/max content examples tested with no loss of content or overflow.
  • All prop combinations work without issue.
  • Tested for FOUC (Flash of Unstyled Content) in both SSR (Server-Side Rendering) and SSG (Static Site Generation) settings.
  • Controlled and uncontrolled input components tested.
  • Props/slots can be updated after initial render.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 11, 2026

View your branch deployment here: https://mi6.github.io/ic-ui-kit/branches/734-tooltip-icon/web-components
View your React branch deployment here: https://mi6.github.io/ic-ui-kit/branches/734-tooltip-icon/react

@github-actions
Copy link
Contributor

github-actions bot commented Feb 11, 2026

Cypress visual tests failed.
View the image diff here: https://github.com/mi6/ic-ui-kit/tree/gh-pages/branches/734-tooltip-icon/cypress-image-diff-screenshots/diff
View the html report here: https://mi6.github.io/ic-ui-kit/branches/734-tooltip-icon/cypress-image-diff-html-report/cypress-image-diff-html-report.html

@GCHQDeveloper416 GCHQDeveloper416 force-pushed the 734-tooltip-icon branch 19 times, most recently from 55c7a47 to 72d64a4 Compare February 17, 2026 11:58
@GCHQDeveloper416 GCHQDeveloper416 force-pushed the 734-tooltip-icon branch 5 times, most recently from 6427e87 to 74d063f Compare February 18, 2026 10:31
@GCHQDeveloper416 GCHQDeveloper416 force-pushed the 734-tooltip-icon branch 11 times, most recently from f7c8cbc to 4e114ef Compare February 19, 2026 14:46
GCHQ-Developer-530
GCHQ-Developer-530 reviewed Feb 20, 2026
View reviewed changes
packages/web-components/src/components/ic-tooltip/ic-tooltip.css
}

:host(.ic-tooltip.ic-tooltip-with-icon) ::slotted([slot="tooltip-icon"]) {
width: var(--ic-space-md);
Copy link
Contributor

@GCHQ-Developer-530 GCHQ-Developer-530 Feb 20, 2026

Choose a reason for hiding this comment

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

I think this might need a 1px (--ic-space-1px) margin top so that it's more in line:
Current implementation:

Image

With 1px top:

Image

GCHQ-Developer-299
GCHQ-Developer-299 requested changes Feb 20, 2026
View reviewed changes
Copy link
Contributor

@GCHQ-Developer-299 GCHQ-Developer-299 left a comment

Choose a reason for hiding this comment

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

Will review once suggestion about top margin is implemented

@GCHQDeveloper416 GCHQDeveloper416 force-pushed the 734-tooltip-icon branch 5 times, most recently from ef4ac4b to 9425478 Compare February 23, 2026 14:32
GCHQ-Developer-299
GCHQ-Developer-299 requested changes Feb 24, 2026
View reviewed changes
Copy link
Contributor

@GCHQ-Developer-299 GCHQ-Developer-299 left a comment

Choose a reason for hiding this comment

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

Spotted a couple of things in the storybooks

packages/react/src/stories/ic-tooltip.stories.jsx Outdated
theme: "inherit",
fixedPositioning: false,
tooltipIconSlot: "tooltip-icon",
tooltipIconAltText: "",
Copy link
Contributor

@GCHQ-Developer-299 GCHQ-Developer-299 Feb 24, 2026

Choose a reason for hiding this comment

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

Can you please give Playground with Icon it's own set of default args? Currently these extra fields are showing up on the regular Playground, where they're not functional

packages/react/src/stories/ic-tooltip.stories.jsx
</svg>
<button aria-describedby="ic-tooltip-1">Native</button>
</IcTooltip>
<IcTooltip
Copy link
Contributor

@GCHQ-Developer-299 GCHQ-Developer-299 Feb 24, 2026

Choose a reason for hiding this comment

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

I'm not sure what's going on here, but the IcButton tooltip doesn't update it's label or icon alt text (the native button is fine)

Screen.Recording.2026-02-24.at.08.46.38.mov

Copy link
Contributor Author

@GCHQDeveloper416 GCHQDeveloper416 Mar 12, 2026

Choose a reason for hiding this comment

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

I have tried to replicate it off develop and the same issue appears when changing the label - screen reader reads the updated label for the first button but the second does not update for screen reader until manually refreshed. Maybe needs another ticket?

@GCHQDeveloper416 GCHQDeveloper416 marked this pull request as draft February 24, 2026 11:47
@GCHQDeveloper416 GCHQDeveloper416 force-pushed the 734-tooltip-icon branch 4 times, most recently from dff9c0e to 090e008 Compare March 12, 2026 09:26
@GCHQDeveloper416 GCHQDeveloper416 marked this pull request as ready for review March 12, 2026 09:50
@GCHQDeveloper416
feat(web-components): add slot for icon on tooltip
4a25a80 
Add slot for icon within tooltip. Prop for alt-text. Update to jest tests. Update to storybook

feat(web-components): tooltip icon playground updated with own args

Tooltip icon playground updated with own args

feat(web-components): add slot for icon on tooltip

Add slot for icon within tooltip. Prop for alt-text. Update to jest tests. Update to storybook

feat(web-components): tooltip icon playground updated with own args

Tooltip icon playground updated with own args

feat(web-components): removal of extra icon-args

removal of extra icon-args
@GCHQDeveloper416
docs(docs): update to docs
c5615ca 
update to docs
@GCHQDeveloper416
feat(react): add react stories and cypress tests for tooltip
658bf95 
Add react stories and cypress tests for tooltip

feat(react): update to default args for react storybook

tooltip playground with icon in storybook has own default args
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@GCHQ-Developer-530 GCHQ-Developer-530 GCHQ-Developer-530 left review comments

@GCHQ-Developer-299 GCHQ-Developer-299 GCHQ-Developer-299 requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@GCHQDeveloper416 @GCHQ-Developer-299 @GCHQ-Developer-530