tinypilot
tinypilot copied to clipboard
tiny-pilot
Use semantic HTML for toggle checkboxes
We use a bit of an unusual style our toggle-style HTML checkboxes:
<div class="toggle-container">
Hide Sensitive Data
<label class="toggle">
<input type="checkbox" id="include-sensitive-data" />
<span class="toggle-slider"></span>
</label>
</div>
Ideally, it would look a little closer to something like:
<label for="include-sensitive-data">Hide Sensitive Data</label>
<input type="checkbox" class="toggle-slider" id="include-sensitive-data" />
In Playwright, we can't select the checkbox through normal methods because the checkbox itself is technically not visible on the screen, so we have to select the <label> element instead. This likely affects users with screen readers as well.
https://github.com/tiny-pilot/tinypilot-pro/blob/d98885ba9b27b574024a394928eaebe737a9cac5/e2e/security.spec.js#L11-L18
Hello @mtlynch , was checking the issue. This link is not working for me. https://github.com/tiny-pilot/tinypilot-pro/blob/d98885ba9b27b574024a394928eaebe737a9cac5/e2e/security.spec.js#L11-L18
@pipliya - It's in our Pro repo, which is not public, but here's what the link points to:
// This is a workaround for our unusual HTML pattern of styling checkboxes as
// toggles. We should use a locator based on the checkbox label once we fix
// https://github.com/tiny-pilot/tinypilot/issues/1638.
const locateToggle = (cssSelector) => {
return page.locator(".toggle").filter({ has: page.locator(cssSelector) });
};
await locateToggle("#require-authentication").click();
I'd like to arrange the HTML so that this Playwright code sets the <input id="include-sensitive-data"> checkbox in the HTML above.
await page.getByLabel("Hide Sensitive Data").setChecked(true);
tldr, I’m not sure we can find a way to maintain our current toggle button implementation and use Playwright’s getByLabel locator.
Background
Our current toggle button implementation is based on a <label> and an <input type=checkbox> element. We didn’t choose that element combination for semantic reasons primarily, but more so because it allows a rather simple implementation of a toggle button without needing any custom JS code. The <input> element’s purpose is mainly for managing the internal state of the checkbox.
Playwright’s getByLabel locator doesn’t seem to be compatible with that approach, because it wants to scroll the corresponding input element into the viewport before interacting with it.
I’ve tried turning the currently “naked” Require username and password text into a <label for="require-authentication">Require username and password</label> element, which correctly associates the label with the checkbox element. I’ve then tried the following Playwright queries – however, all unsuccessful, unfortunately:
-
await page.getByLabel("Require username and password").setChecked(true) -
await page.getByLabel("Require username and password").setChecked(true, { force: true }) -
await page.getByLabel("Require username and password").click() -
await page.getByLabel("Require username and password").click({ force: true }) -
await page.getByLabel("Require username and password").check() -
await page.getByLabel("Require username and password").check({ force: true })
Playwright seems to resolve the checkbox element correctly. However, without the {force: true} option, it fails because the “Target [is] closed”, and with the {force: true} option, it fails because the “Element is outside of the viewport”.
“Target closed” error
Error: locator.setChecked: Target closed
=========================== logs ===========================
waiting for getByLabel('Require username and password')
locator resolved to <input type="checkbox" id="require-authentication"/>
attempting click action
waiting for element to be visible, enabled and stable
element is not visible - waiting...
============================================================
“Element is outside of the viewport” error
Error: locator.setChecked: Element is outside of the viewport
=========================== logs ===========================
waiting for getByLabel('Require username and password')
locator resolved to <input type="checkbox" id="require-authentication"/>
attempting click action
waiting for element to be visible, enabled and stable
forcing action
element is visible, enabled and stable
scrolling into view if needed
done scrolling
============================================================
Discussion
Since this ticket was estimated as “small”, and I wasn’t able to find a promising lead so far, I wanted to discuss our options (at least ones I could think of). I guess it’s mostly a cost<>benefit question in the end.
(1) Turn toggle button into web component
If we wanted to make our toggle button more semantic, we’d probably have to turn it into a web component. That way, we don’t have to use the <input type=checkbox> hack for maintaining internal state, but the state could live in JS code then. That would also allow us to set an aria-checked attribute on the component, for making it accessible to screen readers.
It seems possible to associate <label> elements with custom web components, but I’m not sure whether that would also work with Playwright’s getByLabel in a “native” way.
Maybe worth a try? Converting the current toggle implementation into a web component might require refactoring work down the line, though, as we probably have to adjust client-side JS code, e.g. for wiring event handlers.
(2) Investigate getByLabel locator more deeply
I haven’t understood yet why the getByLabel would fail on interactions if an element is outside of the viewport, whereas our current CSS-based locator seems to succeed given the same circumstances.
The getByLabel locator doesn’t provide a {force: true} option, but maybe there is another way to make it behave similar to our current query approach?
I haven’t used Playwright much, so I’d have to dig deeper here to understand this better.
(3) Extend our current workaround for Playwright compatibility
Maybe we find a way that keeps the current toggle button implementation as is, but that achieves compatibility with Playwright’s expectations. For instance, one thing that I have already tried is to change the checkbox’s CSS in a way that doesn’t hide it directly via opacity: 0, like we do now, but that just moves it away, via position: absolute; left: -99999px. That attempt didn’t work out, but maybe we’d be able to find another such technique that does the trick. I don’t have anything in mind specifically right now, but we might find an approach by poking around.
@jotaen4tinypilot - Thanks for checking into this!
I thought this would be more straightforward, so given the complexity, it's probably not worth diving into right now. Our current Playwright workaround isn't too ugly.
If/when we pick this back up, I think the most promising path is turning this into a web component.
This blog post gave an example of a JS-free toggle switch that's pretty close to what we're already doing:
https://www.htmhell.dev/adventcalendar/2023/2/#custom-switches
Let's give this an hour and see if we can adopt that solution. If not, we'll punt on this.