podman-desktop
podman-desktop copied to clipboard
chore: use checkbox component
What does this PR do?
I noticed several places where we're using a standard blue instead of our styled Checkbox.
Run Image is the biggest one, just requires the ability to set a class on Checkbox for spacing.
Screenshot / video of UI
Before:
After:
What issues does this PR fix or reference?
N/A
How to test this PR?
Unit tests still work; page through Run Image form to confirm styling.
@odockal helped investigate. Although for humans and vitest it's fine, the E2E tests are unable to click a Checkbox because it's a native sr-only checkbox underneath a FA SVG - to playwright, there is always something hiding the real checkbox. We will likely have to rewrite the Checkbox to avoid this.
@deboer-tim A workaround could be in this case, to use parent element (div) that has a label for specific checkbox, so we can get the element with page.getByLabel('CheckBox name') and make the test pass again. Although, anyone writing the test trying to handle the checkbox will be facing the same issue over and over, so proper handling as rewrite the checkbox seems like viable solution.
converting to draft
Rebased with changes to checkbox, moving back to ready for review. @benoitf, @lstocchi (or others) please take another look or re-confirm your approval by merging. Thanks to @odockal for his help.
This change originally caused failures in the E2E tests because our Checkbox was incompatible with Playwright, since the input in this checkbox component was neither visible nor Aria compliant. I wasted lots of time trying to build a custom checkbox that was aria-compliant, but the fix was much simpler after carefully reading the Playwright requirements here: https://playwright.dev/docs/actionability#visible
The input checkbox cannot be hidden or 0 size, and cannot be 'behind' the div icon. However, it can be opacity-0 and on top using absolute positioning and changing the z-order by swapping the order of elements.
@benoitf are you ok to merge?
yes