feat(PPDSC-2118): tooltip pointer #217
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- <p> cannot contain the <div> required for the pointer
- add optional pointer element too tooltip - add default offset to tooltip with ability to override
mstuartf
requested review from
jps,
Xin00163,
pdimova,
baburay23,
mutebg,
Vanals,
evgenitsn and
Srinivasan-Ramasamy
as code owners
|
You can preview these changes on: |
- pointer and panel should both inherit background colour from tooltip style preset - panel should have additional style preset to manage border radius and text colour
mstuartf
added
ready for review
mutebg
suggested changes
Jun 3, 2022
- Move inset styling to styled component - Update inset styling based on offset passed - Handle non-px and MQ values for offset (distance) - Update tests
mutebg
reviewed
Jun 7, 2022
mutebg
previously approved these changes
Jun 7, 2022
Xin00163
reviewed
Jun 7, 2022
There was a problem hiding this comment.
Could we add the aria-hidden attribute to tooltip like https://www.w3.org/WAI/WCAG21/Techniques/aria/ARIA1? This is something I missed in the last pr. When scanning through with screenreader, the content of the tooltip is read twice. By adding this, we can avoid redundancy. Happy to have a chat about this.
- To prevent screen readers reading tooltip content twice (once when focusing on the context element and once when scanning).
mutebg
approved these changes
Jun 8, 2022
Xin00163
approved these changes
Jun 8, 2022
Xin00163
pushed a commit
that referenced
this pull request
Oct 17, 2022
* feat(PPDSC-2118): change tooltip element to div - <p> cannot contain the <div> required for the pointer * feat(PPDSC-2118): add pointer to tooltip - add optional pointer element too tooltip - add default offset to tooltip with ability to override * feat(PPDSC-2118): make sure pointer id is globally unique * feat(PPDSC-2118): add test case to make meet coverage req * feat(PPDSC-2118): update default tooltip trigger to include focus * feat(PPDSC-2118): split tooltip panel style preset - pointer and panel should both inherit background colour from tooltip style preset - panel should have additional style preset to manage border radius and text colour * feat(PPDSC-2118): add no pointer scenario * feat(PPDSC-2118): update tests to assert on tooltip position * feat(PPDSC-2118): separate tooltip panel element * feat(PPDSC-2118): apply tooltip offset in styled component - Move inset styling to styled component - Update inset styling based on offset passed - Handle non-px and MQ values for offset (distance) - Update tests * feat(PPDSC-2118): nest tooltip panel and pointer defaults * feat(PPDSC-2118): set background-color separately for panel and pointer * feat(PPDSC-2118): update theme snapshots * feat(PPDSC-2118): stop x and y coords being passed to html * feat(PPDSC-2118): rename tooltip title prop to content * feat(PPDSC-2118): change showPointer to hidePointer * feat(PPDSC-2118): add tests for offset logic utils * feat(PPDSC-2118): add aria-hidden to tooltip - To prevent screen readers reading tooltip content twice (once when focusing on the context element and once when scanning).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PPDSC-2118
What
I have done:
I have tested manually:
Differences to ticket:
distanceas prop and an override. I only included it as an override as this seems more appropriate.Other notes:
<div>rather than a<p>. This is because the pointer<div>needs to be a child of the tooltip so that it can be positioned correctly. But<div>cannot appear as a descendant of<p>(or we get avalidateDOMNestingconsole error).