DT 40 Tooltip tests #178
DT 40 Tooltip tests #178
Conversation
…ooltip-DT-40-tests # Conflicts: # components/tooltip/tooltip-tippy-headless.vue
| }, | ||
|
|
||
| data () { | ||
| return { | ||
| TOOLTIP_KIND_MODIFIERS, | ||
| tip: null, | ||
| placement: '', | ||
| localShow: false, |
There was a problem hiding this comment.
during writing tests localShow property was added for reactivity purpose as well (like show property)
There was a problem hiding this comment.
Hey Nick, can you explain your reasons for the localShow here? Is it only needed for testing purposes? We usually try to find ways to avoid local copies of props as it adds complexity.
Seems like it's purpose is when we get an onMount or onHide event from tippy we change localShow to remain in sync with the results of this event. I'm not sure if this is necessary as if I'm not mistaken this.$emit('update:show'); should update the show prop if the consumer is using .sync right? Is it just in vue-test-utils this isn't happening?
There was a problem hiding this comment.
hey Brad,
right, localShow was added for testing purpose and also it was preventing using mandatory sync modifier for show prop.
show prop is required now, tooltip component doesn't have local state for show variable, and this means parent should manage show prop with sync modifier (otherwise a11y will not work)
The changes are done, but now I'm thinking may be local state will be helpful?
for example after this change 12 show variables are needed for page where all possible tooltip directions defined.
There was a problem hiding this comment.
hm, in this case may be aria label (here) could be set tofalse, then tooltip will be accessible all the time. Tests will need to be updated as well.
There was a problem hiding this comment.
Yeah I think this should be fine
There was a problem hiding this comment.
great, tooltip got aria-hidden="false" by default and aria related tests were aligned
| }, | ||
|
|
||
| data () { | ||
| return { | ||
| TOOLTIP_KIND_MODIFIERS, | ||
| tip: null, | ||
| placement: '', | ||
| localShow: false, |
There was a problem hiding this comment.
Hey Nick, can you explain your reasons for the localShow here? Is it only needed for testing purposes? We usually try to find ways to avoid local copies of props as it adds complexity.
Seems like it's purpose is when we get an onMount or onHide event from tippy we change localShow to remain in sync with the results of this event. I'm not sure if this is necessary as if I'm not mistaken this.$emit('update:show'); should update the show prop if the consumer is using .sync right? Is it just in vue-test-utils this isn't happening?
…-absolute-positioning' into component-tooltip-DT-40-tests # Conflicts: # components/tooltip/tooltip-tippy-headless.vue # components/tooltip/tooltip_tippy.mdx # components/tooltip/tooltip_tippy_default.story.vue # components/tooltip/tooltip_tippy_flip.story.vue
nikitadialpad
merged commit f18dea0
into
components-Tooltip-Popover-DT-40-absolute-positioning
) * DT 40 Absolute positioning for tooltips (#131) * DT 40 Absolute positioning for popovers & tooltips * updates * updates * update tooltip * update tooltip * clean up * refactoring, clean up * resolve comments * resolve comments refactor first iteration stories * update comments * DT 40 Tooltip documentation (#177) * add documentation for Tippy tooltip * update reactive "show" * resolve comments, update eslint config because of an error * update esplint json * DT 40 Tooltip tests (#178) * add documentation for Tippy tooltip * update reactive "show" * update tests * update tests * resolve comments * resolve comments * feat: DT 40 Absolute positioning for popover (#135) * DT 40 Absolute positioning for popovers & tooltips * updates * updates * update tooltip * update tooltip * clean up * refactoring, clean up * resolve comments * popover implementation, decomposition tippy, clean up * add animation, clean up * clean up popover * clean tooltip folder * add import * clean up, updates * DT-40 popover component, refactoring after tooltip implementation * resolve comments * clean up * resolve comments * fix eslint * leftover after merge * leftover after merge * Added test for tooltip tippy; Update components DtLazyShow, popover, tooltip; * chore: update package-lock.json * refactor: after merge * story doc * update story doc * add flip for popover * add implementation "modal" * remove v-click-outside * update documentation * update stories * prevent closing popup when a user blur and focus repeatedly during transition * clean up * update storybook folder for tooltip. * added comment for after-leave event * docs: update * resolve comments * fix typo * update overlay, tests * resolve comments * add comment * popover uv alignments * clean up, update data * clean up, remove prev version tooltip. * update docs * update z-index for tooltip * update border-box for content * add width content prop (anchor), update focus logic, * clean up, rename contentWidth * set empty string to content width by default * package lock * restore tooltip test * clean up tooltip test add new constant to popover update default story * add flushPromises to utils * add $nextTick before setPopoverContentAnchorWidth * update tooltip story Co-authored-by: Brad Paugh <brad.paugh@dialpad.com>
# [1.6.0](v1.5.0...v1.6.0) (2021-11-25) ### Bug Fixes * add our alpha sort to list-item. Change slot types to VNode for consistiency ([#202](#202)) ([686b2b9](686b2b9)) ### Features * components tooltip and popover. absolute positioning (dt 40) ([#194](#194)) ([dd9e44d](dd9e44d)), closes [#131](#131) [#177](#177) [#178](#178) [#135](#135)
DT 40 Tooltip tests
Changes includes alignments for tests after tippy tooltip integration.
Added hot fix
waitForTippyUpdatefunction to fix main issue with testing - firing and handling user events in the test.🛠️ Type Of Change
📖 Description
Tippy tooltip test
📝 Checklist
🔮 Next Steps
According to that changes, I think it's a question now for popover component - do we need to test it? previous implementation doesn't have tests.
📷 Screenshots / GIFs