Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[QE testing] form field selectors: align on using id where possible to simplify QE tests #443

Open
mturley opened this issue Sep 28, 2022 · 3 comments
Open

[QE testing] form field selectors: align on using id where possible to simplify QE tests #443

mturley opened this issue Sep 28, 2022 · 3 comments
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. priority/backlog Higher priority than priority/awaiting-more-evidence. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@mturley
Copy link
Collaborator

mturley commented Sep 28, 2022 •
edited
Loading

Related to #198. Ideally we can fix both together.

For rationale, see the "Accessibility and QE selectors" section at the bottom of the description in #404. The main conclusions there are:

  • We need a unique id attribute on each form input in order to provide accessible labels without redundant aria-label attributes
  • We should NOT use aria-label as a selector for QE tests since it is user-facing text that is subject to change or be removed (it is used by screen-readers for accessibility)
  • QE tests should use id selectors where possible and work with dev on what selector to use when id cannot be provided on a case-by-case basis. When this comes up it is likely a bug in PatternFly and we can provide a real id once that bug is addressed.

#404 included changes to support the above for the Proxy settings form and the Identity form (the Create/Edit Credential modal in the Admin view). That PR includes heavy refactoring that should not be backported to 2.1.x, but we should create a separate PR to backport matching id / aria-label changes to 2.1.2 once that release branch exists. We should then make sure we follow up to help transition relevant QE tests to use the new id selectors.

As more forms are refactored to use the new react-hook-form components, we will also be aligning their field attributes in a similar way, and any of those that are merged before 2.1.2 code freeze should also have their selector changes backported as part of this issue.
@mturley mturley changed the title Form field unique selectors: align on using id where possible, backport QE-breaking changes to 2.1.2, follow up to fix QE tests Form field unique selectors: align on using id where possible, backport QE-test-breaking changes to 2.1.2, follow up to fix QE tests Sep 28, 2022
@mturley mturley changed the title Form field unique selectors: align on using id where possible, backport QE-test-breaking changes to 2.1.2, follow up to fix QE tests Form field unique selectors: align on using id where possible, backport QE-test-breaking changes to 2.1.2 (selector attributes only), follow up to help fix QE tests Sep 28, 2022
@mturley
Copy link
Collaborator Author

mturley commented Sep 28, 2022

PRs containing changes that will break QE tests, which should have custom backport PRs to align the 2.1.2 branch to use their new field selectors:

Please edit this comment to add more if you identify them.

@mturley mturley changed the title Form field unique selectors: align on using id where possible, backport QE-test-breaking changes to 2.1.2 (selector attributes only), follow up to help fix QE tests Form field unique selectors: align on using id where possible, follow up to help fix QE tests Oct 5, 2022
@mturley mturley added the v3.0.0 label Oct 5, 2022
@ibolton336 ibolton336 mentioned this issue Dec 14, 2022
Closed
@ibolton336 ibolton336 mentioned this issue Jan 10, 2023
Merged
@ibolton336 ibolton336 mentioned this issue Apr 20, 2023
Open
@mturley mturley mentioned this issue Jun 6, 2023
Merged
@ibolton336 ibolton336 reopened this Jun 12, 2023
@ibolton336 ibolton336 moved this to Todo in Konveyor UI Jun 22, 2023
@ibolton336 ibolton336 added the triage/accepted Indicates an issue or PR is ready to be actively worked on. label Jun 22, 2023
This was referenced Sep 13, 2023
Closed
Merged
@gildub
Copy link
Contributor

gildub commented Sep 13, 2023 •
edited
Loading

QE Requests (7 Sept 2023 - initially from #1336) :

  • Cancel button ID to be 'cancel' across the UI.
  • Submit button ID to be 'submit' across the UI.
  • Pages with top table level dropdown kebab with latter to be identified for example with id="toolbar-kebab"
    • applications-table-assessment.tsx
    • applications-table-analysis.tsx
    • migration-waves.tsx
  • Pages with table rows having action components, such as "edit" (pencil icon) or "actions" (kebab with dropdown) must have a selector. Note there is no need to be uniquely named across rows.
    • applications-table-assessment.tsx
    • applications-table-analysis.tsx
    • migration-waves.tsx

@gildub gildub moved this from Todo to In Progress in Konveyor UI Sep 13, 2023
@gildub gildub moved this from In Progress to Todo in Konveyor UI Sep 13, 2023
ibolton336 added a commit that referenced this issue Sep 15, 2023
@gildub @ibolton336
✨ Selectors to uniquely identify dropdowns (#1345)
35fd96a 
@ibragins explained QE needs : 
- A consistent selector across the UI to identify kebab in toolbar, for
instance id="toolbar-kebab"
- A consistent selector across the UI to distinct, on a given table row
between, between a "pencil" button and the "actions" kebab. There is no
need to uniquely identify those selector by row, QE already has
selectors for that purpose.

We can see those IDs in action in current PR in
applications-table-assessment.tsx.

Meanwhile that cannot be easliy added to the legacy tables (PF4 or PF5
deprecated table) because we don't have access to the wrapper component.
Therefore will have to wait for legacy table migration to be finished
first.

This applies to every page having a top kebab (Toolbar) and rows with
several actions button/kebab:
- [x] applications-table-assessment.tsx
- [x] applications-table-analysis.tsx
- [x] migration-waves.tsx

On the MigrationWaves page, the deprecated dropdown has been replaced
with PF5 version.

#443

---------

Signed-off-by: Gilles Dubreuil <gdubreui@redhat.com>
Co-authored-by: Ian Bolton <ibolton@redhat.com>
@dymurray dymurray moved this to 📋 Backlog in Planning Sep 15, 2023
@sjd78 sjd78 removed the v3.0.0 label May 31, 2024
@konveyor-ci-bot konveyor-ci-bot bot added needs-kind Indicates an issue or PR lacks a `kind/foo` label and requires one. needs-priority Indicates an issue or PR lacks a `priority/foo` label and requires one. labels May 31, 2024
@sjd78 sjd78 added priority/backlog Higher priority than priority/awaiting-more-evidence. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. and removed needs-kind Indicates an issue or PR lacks a `kind/foo` label and requires one. needs-priority Indicates an issue or PR lacks a `priority/foo` label and requires one. labels Jun 11, 2024
@sjd78 sjd78 added the good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. label Aug 21, 2024
@konveyor-ci-bot
Copy link

This issue has been marked 'good first issue'
Please, make sure it aligns with the criteria found here

@sjd78 sjd78 changed the title Form field unique selectors: align on using id where possible, follow up to help fix QE tests [QE testing] form field selectors: align on using id where possible to simplify QE tests Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. priority/backlog Higher priority than priority/awaiting-more-evidence. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
Planning
Status: 📋 Backlog
Konveyor UI
Status: Todo
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mturley @gildub @sjd78 @ibolton336