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
Cypress: Updated isPseudoLocalized() to correctly process multiple elements #9385
Cypress: Updated isPseudoLocalized() to correctly process multiple elements #9385
Conversation
cy.wrap(subject).each(($el) => { | ||
const text = $el.text(); | ||
if (text.length > 0) { | ||
expect(text).to.match(/\[[^a-zA-Z]+\]/); | ||
} | ||
}); |
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.
tested this change locally, nice!
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.
looks good to me!
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 looks great! Just one comment about passing the TFunction around since it's something Sam dinged me for a lot. :)
cy.byTestID('application-launcher-item').isPseudoLocalized(); | ||
// wait for both console help menu items and additionalHelpActions items to load | ||
// additionalHelpActions come from ConsoleLinks 'HelpMenu' yaml and are not translated | ||
cy.get('.pf-c-app-launcher__group').should('have.length', 2); |
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.
We should open a Bugzilla for the menu items not translated. I think it would be better to move this into console so they can be translated instead of leaving them as ConsoleLink resources, although it will require some operator changes.
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.
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.
I added some examples here based on what we've done elsewhere. If you remove the t argument and adjust those lines of code, this should be all set.
3fdd2bf
to
9c930b7
Compare
apologies @rebeccaalpert, looks like I didn't push the fixes :-) Should be updated now. |
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.
Looks good to me! Thank you.
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dtaylor113, rebeccaalpert, rhamilto The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Previously a call such as:
cy.byTestID('nav').isPseudoLocalized();
would incorrectly result in one call to
isPseudoLocalized()
which would test a single combined text string consisting of all of the text of matching elements! In this example "[Ḥṓṓṃḛḛ] [ṎṎṽḛḛṛṽḭḭḛḛẁ] [Ṕṛṓṓĵḛḛͼţṡ]...[ṎṎṗḛḛṛααţṓṓṛṡ] [Ḉṓṓṃṗṵṵţḛḛ]..." -instead of testing each element text individually. So,isPseudoLocalized()
would pass as long as first and last elements were translated.Updated
isPseudoLocalized()
to correctly process multiple elements, as well as:pseudolocalization.spec.ts
to correctly test masthead Help menu items and Overview dashboard 'Cluster utilization' card.data-test
ids