-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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.
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
feat: IATR-M0 Page Header (#24722) #25360
feat: IATR-M0 Page Header (#24722) #25360
Conversation
Test summaryRun details
View run in Cypress Dashboard ➡️ Flakiness
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to account for multiple "MIDDLE" parts being hidden by the ellipsis
@@ -197,6 +197,9 @@ describe('<DebugFailedTest/>', () => { | |||
|
|||
assertRowContents(testResult) | |||
|
|||
cy.contains('...').realHover() | |||
cy.contains('[data-cy=tooltip-content]', 'Test content 2').should('be.visible') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should contain everything that was hidden by the ellipsis separated by >
cy.contains('[data-cy=tooltip-content]', 'Test content 2').should('be.visible') | |
cy.contains('[data-cy=tooltip-content]', 'Test content 2 > Test content 3 > Test content 4').should('be.visible') |
@@ -30,7 +30,17 @@ | |||
titlePart.type === 'LAST-1' ? 'shrink-0 whitespace-pre' : | |||
titlePart.type === 'LAST-0' ? 'pl-2.5 truncate' : 'px-2.5 truncate'" | |||
> | |||
{{ titlePart.title }} | |||
<template v-if="titlePart.title === '...'"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<template v-if="titlePart.title === '...'"> | |
<template v-if="titlePart.type === 'ELLIPSIS'"> |
Check with type instead of title
@@ -121,6 +133,7 @@ const failedTestData = computed(() => { | |||
{ | |||
title: '...', | |||
type: 'ELLIPSIS', | |||
originalTitle: ele, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will need to gather up all the "MIDDLE" parts to construct the originalTitle
passed to the ELLIPSIS tooltip
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense, thanks! I think I was missing the underlying concept here, that it's really combining the parts 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updates look great. I really like the refactor for some of the enum names and variable names
User facing changelog
NA, merging to feature branch
Additional details
Added an optional
originalTitle
property to theMappedTitlePart
type. There are numerous other ways to handle this but this one seems fine to me. We could maybe work on the readability of this component at a future time.Steps to test
Visit the component test for
DebugFailedTest.vue
and let it run. Observe tooltip appears when you hover over the ellipses:Sections that are truncated by ellipses, but still visible, will not have this tooltip, based on the wording of the original issue. To do that we would need a little more sophisticated approached to determine that the browser has truncated the text and conditionally enable the tooltip, the presence of the class alone is not enough. So that is out of scope for this ticket.
How has the user experience changed?
PR Tasks
cypress-documentation
?type definitions
?