Skip to content

Commit

Permalink
Fix the sidebar button on iPhones
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
dscho committed Sep 29, 2024
1 parent 8aa0d06 commit 7d3b019
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 2 deletions.
2 changes: 1 addition & 1 deletion assets/sass/sidebar.scss
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ aside.sidebar.active {
background-color: $black-3 !important;
display: block;
position: fixed;
padding: 2rem 0.4rem;
padding: 2rem 0rem;
z-index: 1;
border: none;
left: 0;
Expand Down
2 changes: 1 addition & 1 deletion layouts/partials/sidebar.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{{ $section := .Scratch.Get "section" }}
<button class="sidebar-btn"></button>
<div tabindex="1" class="sidebar-btn"></div>
<aside class="sidebar" id="sidebar">
<nav>
<ul>
Expand Down
7 changes: 7 additions & 0 deletions playwright.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ module.exports = defineConfig({
name: 'chrome',
use: { ...devices['Desktop Chrome'], channel: 'chrome' },
},
{
name: 'iPhone',
use: {
...devices['iPhone 11'],
},
},


/* Test against mobile viewports. */
// {
Expand Down
16 changes: 16 additions & 0 deletions tests/git-scm.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -265,3 +265,19 @@ test('404', async ({ page }) => {
// the usual navbar is shown
await expect(page.getByRole('link', { name: 'Community' })).toBeVisible()
})

test('sidebar', async ({ page }) => {
await page.goto(`${url}community`);

test.skip(!await page.evaluate(() => matchMedia('(max-width: 940px)').matches),
'not a small screen');

const sidebarButton = page.locator('.sidebar-btn');
await expect(sidebarButton).toBeVisible();
await sidebarButton.click();

const about = page.getByRole('link', { name: 'About', exact: true });
await expect(about).toBeVisible();
await about.click();
await expect(page.getByRole('heading', { name: 'About - Branching and Merging' })).toBeVisible();
});

0 comments on commit 7d3b019

Please sign in to comment.