Scope accordion-item search to expanded panel only #9531
Conversation
cypress/README.md
Outdated
|
|
||
| * `cy.accordion(title)` - open an accordion panel. `title`: String for the accordion title for the accordion panel to open. | ||
| * `cy.accordionItem(name)` - click on a record in the accordion panel. `name`: String for the record to click in the accordion panel. | ||
| * `cy.selectAccordionItem(accordionPath)` - navigates the accordion panel & expand the nodes along the given path and click the final target item. |
There was a problem hiding this comment.
This is great! Can you add examples of the accordionPath? Are there gotchas with making mistakes in the intermediate paths to the desired node? Do the errors make it clear when the intermediate section node wasn't found versus the desired final target item?
jrafanie
left a comment
jrafanie
left a comment
There was a problem hiding this comment.
My only comment is the example seen above. Otherwise, this is looking really good.
| */ | ||
| Cypress.Commands.add('selectAccordionItem', (accordionPath) => { | ||
| cy.get('li.list-group-item').then(($items) => { | ||
| cy.get('div.panel-collapse.collapse.in li.list-group-item').then(($items) => { |
There was a problem hiding this comment.
Scope the search into the expanded tree only using container class-name, expanded the class will be: panel-collapse collapse in
Nice find!
asirvadAbrahamVarghese
force-pushed
the
enhance-select-accordion-item
branch
from
04c8d69 to
33a3bc4
Compare
| // If we reach here, it means the label was not found | ||
| throw new Error(`Accordion item: "${accordionLabel}" not found`); | ||
| const errorMessage = `${ | ||
| isClickableNode ? 'Target' : 'Intermediate' |
There was a problem hiding this comment.
This is very nice. Before, I can imagine someone could get confused if they had a typo in the parent section node but not in the target/leaf node, so making it clear when the target or intermediate was not found should help make that clear.
@GilbertCherrie @elsamaryv Are you good with the names here? Do we have language for the target node and the parent nodes? In other words, intermediate is region and zone below and the target is the server. In the backend, we might call them parent nodes and the target is the leaf node, similar to the language of binary trees.
There was a problem hiding this comment.
yeah maybe try to match the names in the code, I think it might be parent and child but not too sure
There was a problem hiding this comment.
I'm fine with Target/Intermediate for now. I don't really have a better name.
From above:
cy.selectAccordionItem([ 'ManageIQ Region: Region 0 [0]', 'Tenants Section', // This intermediate node is not valid 'My Company',
'Intermediate node "Tenants Section" was not found' make sense to me. For followup, do we have the context on which Accordion we're in? Would it make sense to add it to the error? I'm fine with what we have for now. If it makes sense, you can do a followup @asirvadAbrahamVarghese
cypress/README.md
Outdated
| Path with regular expressions: | ||
| cy.selectAccordionItem([/^ManageIQ Region:/, /^Zone:/, /^Server:/]); | ||
| Mixed path with strings and regular expressions: | ||
| cy.selectAccordionItem([/^ManageIQ Region:/, 'Zones', /^Zone:/]); |
There was a problem hiding this comment.
We might have to revisit this for the formatting. I'm fine with it for now but other examples in this guide are inline and use e.g.. We can decide how to normalize them later though.
There was a problem hiding this comment.
Understood, updated to keep it in line with the current structure - 1a38a85
asirvadAbrahamVarghese
force-pushed
the
enhance-select-accordion-item
branch
from
a0c9dfa to
bf15ecf
Compare
jrafanie
left a comment
jrafanie
left a comment
There was a problem hiding this comment.
LGTM, minor question for followup PR if it makes sense.
| // If we reach here, it means the label was not found | ||
| throw new Error(`Accordion item: "${accordionLabel}" not found`); | ||
| const errorMessage = `${ | ||
| isClickableNode ? 'Target' : 'Intermediate' |
There was a problem hiding this comment.
I'm fine with Target/Intermediate for now. I don't really have a better name.
From above:
cy.selectAccordionItem([ 'ManageIQ Region: Region 0 [0]', 'Tenants Section', // This intermediate node is not valid 'My Company',
'Intermediate node "Tenants Section" was not found' make sense to me. For followup, do we have the context on which Accordion we're in? Would it make sense to add it to the error? I'm fine with what we have for now. If it makes sense, you can do a followup @asirvadAbrahamVarghese
4 participants
Before:
When identical node names appear across multiple trees in the sidebar, regardless of whether they're expanded or collapsed:
Settings -> ManageIQ Region: Region 0 [0] -> Zone: Default Zone (current) -> Server: EVM [1] (current)
Diagnostics -> ManageIQ Region: Region 0 [0] -> Zone: Default Zone (current) -> Server: EVM [1] (current)
if the intended tree is under Diagnostics but the first match appears under Settings, it ends up selecting the one from Settings
After:
Scope the search into the expanded tree only using container class-name, expanded the class will be:
panel-collapse collapse inExpanded
if not, it will be
panel-collapse collapseThis is the case for most of the accordion panels, verified in these:
Overview -> utilization
Services -> Catalogs
Services -> Workloads
Automation -> Embedded Automate -> Explorer
Automation -> Embedded Automate -> Generic Objects
Automation -> Embedded Automate -> Customization
Settings -> Application Settings
@miq-bot assign @jrafanie
@miq-bot add-label cypress
@miq-bot add-label test