Fix sidebar button on iPhones #1891
Conversation
This version brings four regression fixes (for full details, see https://github.com/microsoft/playwright/releases/tag/v1.47.2 and https://github.com/microsoft/playwright/releases/tag/v1.47.1). While the fixed regressions do not affect the current Playwright tests in git-scm.com, the `page.pause()` and Trace Viewer fixes affect interactive development and debugging. Signed-off-by: Johannes Schindelin <[email protected]>
As per https://webkit.org/b/22261#c68, the Safari browser never gives buttons the focus. This breaks the expectation of git-scm.com's sidebar button that is shown on small screens: clicking on it will never open that sidebar and navigation is made harder than necessary by that. The solution, as per the hint at the incredibly helpful post at https://stackoverflow.com/questions/42758815/safari-focus-event-doesnt-work-on-button-element#53157834 is to turn the <button> into a <div tabindex="1">. That way, it works even on iPhones. With minor style adjustments, the button even looks the same as before! Reported-by: Toon Claes <[email protected]> Signed-off-by: Johannes Schindelin <[email protected]>
|
@dscho Should this trigger an automatic deploy? |
It did: https://github.com/git/git-scm.com/actions/runs/11108736284 You may not immediately see any improvement, as Cloudflare's caches have to drain first, and in addition your browser seems to be directed via a |
And I just verified that it works, by running |
3 participants
Changes
This makes the sidebar button work when browsing on iPhones.
Context
On small screens, the navigation sidebar is hidden by default, and instead a button is displayed on the left side:
The idea is that the navigation sidebar is displayed when tapping that button. However, as @To1ne pointed out to me at GitMerge, tapping this sidebar button on iPhones does not do anything.
The reason is that Safari does not give focus to
<button>s, and the sidebar button is a<button>.With this PR, the button is a
<div>with a tab index, and therefore can receive focus on iPhones. The result will now look like this:For good measure, I added a Playwright test; This test is not run in CI by default (to avoid the cost of downloading Webkit in every CI/PR build), but can at least be run manually.