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.

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

✨ Selectors to uniquely identify dropdowns #1345

Merged
merged 5 commits into from
Sep 15, 2023
Merged

✨ Selectors to uniquely identify dropdowns #1345

merged 5 commits into from
Sep 15, 2023

Conversation

gildub
Copy link
Contributor

@gildub gildub commented Sep 11, 2023

@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:

  • applications-table-assessment.tsx
  • applications-table-analysis.tsx
  • migration-waves.tsx

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

#443

@codecov
Copy link

codecov bot commented Sep 11, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (2ec8184) 41.43% compared to head (92dfb2d) 41.43%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1345   +/-   ##
=======================================
  Coverage   41.43%   41.43%           
=======================================
  Files         137      137           
  Lines        4279     4279           
  Branches     1026     1026           
=======================================
  Hits         1773     1773           
  Misses       2418     2418           
  Partials       88       88           
Flag Coverage Δ
client 41.43% <ø> (ø)
server ∅ <ø> (∅)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gildub gildub changed the title ✨ Ids for top kebab dropdown ✨ [WIP] Ids for top kebab dropdown Sep 11, 2023
@gildub gildub changed the title ✨ [WIP] Ids for top kebab dropdown ✨ [WIP] Selectors to uniquely identify dropdowns Sep 11, 2023
@gildub gildub self-assigned this Sep 11, 2023
Signed-off-by: Gilles Dubreuil <gdubreui@redhat.com>
Signed-off-by: Gilles Dubreuil <gdubreui@redhat.com>
…ns IDs

Signed-off-by: Gilles Dubreuil <gdubreui@redhat.com>
@gildub gildub changed the title ✨ [WIP] Selectors to uniquely identify dropdowns ✨ Selectors to uniquely identify dropdowns Sep 15, 2023
@ibolton336 ibolton336 merged commit 35fd96a into konveyor:main Sep 15, 2023
6 checks passed
@ibolton336
Copy link
Member

@istein1

@sshveta
Copy link

sshveta commented Sep 18, 2023

Very helpful PR @ibolton336 . Thanks for these changes.

ibolton336 pushed a commit that referenced this pull request Sep 25, 2023
…sExpandable prop in useTableControlProps (#1398)

In #1345, we changed the structure of the expandable rows in the
Migration Waves table and caused the `isExpanded` prop to be present on
all rows (which should only be on the expanded content rows), which
caused all rows to be hidden because it was false. This PR restores the
intended structure: Each row is wrapped in a `<Tbody>` with the
`isExpanded` prop, which contains two `<Tr>`s: one for the row itself
and one for the expanded content.

Also removes the `cursor: "pointer"` inline style on the `Tbody` which
was causing the whole row to appear clickable even though only the
expandable cells are clickable.

Signed-off-by: Mike Turley <mike.turley@alum.cs.umass.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants