Skip to content

Responsive browser chrome #452

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

Merged
merged 3 commits into from
May 30, 2023

Conversation

eliot-akira
Copy link
Collaborator

@eliot-akira eliot-akira commented May 25, 2023

What?

Adds responsive breakpoints and styles to fit browser chrome to a range of screen widths.

Why?

Resolves #433

How?

localhost_5400_902px

localhost_5400_812px

localhost_5400_620px

localhost_5400_420px

Testing Instructions

  1. Check out the branch.
  2. Run npm run dev.
  3. Visit http://localhost:5400/ and view the website at various screen widths.

@eliot-akira eliot-akira requested review from ellatrix and adamziel May 25, 2023 19:18
Copy link
Collaborator

@adamziel adamziel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is great @eliot-akira! I've been just showing Playground to @ellatrix yesterday and it didn't look right on her phone at all. Now it will!

It would be even more useful on mobile phones if there was no body padding at all and the entire viewport was occupied by WordPress and the address bar. What do you think? It's not a blocker here at all, but it would be nice to have.

@eliot-akira
Copy link
Collaborator Author

How's this? It takes up full viewport for tablet (roughly) and below. I also reduced the side padding for the toolbar to align better with WordPress admin menu. Probably someone with more design chops can refine the details later.

localhost_5400_740px

localhost_5400_470px

@adamziel
Copy link
Collaborator

Fantastic! I found a small glitch on an iPhone – the "address bar area" has a top padding but no bottom padding which looks a bit off – it would be great to add a little bit of space between the selects and the bottom of the top bar:

CleanShot 2023-05-27 at 00 57 30@2x

Otherwise it looks great!

@eliot-akira
Copy link
Collaborator Author

eliot-akira commented May 27, 2023

the "address bar area" has a top padding but no bottom padding which looks a bit off – it would be great to add a little bit of space between the selects and the bottom of the top bar

Oh, I see - I missed the lack of bottom padding for the toolbar, because so far I'd only been visually testing while the user is logged in, as is default when starting to develop on localhost:5400. The WP admin menu top bar has the same (or similar?) background color as the Playground toolbar.

Here's what it looked like previously. The Playground toolbar having no bottom padding, with the WP admin top bar:

localhost_5400__theme=twentytwenty mobile toolbar with no bottom padding - logged in

On the left side, there are three horizontal elements: the address bar, PHP version selector, and the WP admin dashboard icon. The first two are part of the Playground, and the last one is within the iframe. The padding on the top/bottom, left/right, and the gaps in between elements, are all 10px.

The Playground toolbar and WP admin menu look like they're parts of a single interface component. I checked and found that the toolbar has background color #1e2327 but the admin menu is #1d2327. Super subtle difference.

I wonder if they should be made the same color, so when the user is logged in, the toolbar looks like an extension of the admin menu. On the other hand, maybe the toolbar should have a more distinct background color to clarify that it's a different component.


By the way, navigating the toolbar by keyboard, it feels seamless to switch focus from toolbar input and select elements, to the WP admin menu, then to the site header menu.

That made me think about accessibility - checking each part of the toolbar:

  • Address bar *
  • PHP and WordPress version selectors *
  • Import/export icons

Those marked with * could use ARIA labels (and maybe roles?).


When the user is logged out:

localhost_5400__theme=twentytwenty mobile toolbar with no bottom padding - logged out

Right, I removed the top margin of the address bar, and made the entire toolbar have padding 10px all around.

localhost_5400__theme=twentytwenty mobile toolbar with padding - logged out

That's well and good.


But when the user is logged in:

localhost_5400__theme=twentytwenty tablet toolbar with padding - logged in

The toolbar's bottom padding and the WP admin menu's top padding combine to make a gap of 20px.

localhost_5400__theme=twentytwenty mobile toolbar with padding - logged in

Compared to previously - the first screenshot in this comment - doesn't it feel less balanced, like there's too much gap between the PHP version selector and the admin menu icons?

Maybe if the toolbar had a more distinct color, it would look like its own grouped component with consistent padding and gaps, instead of merging with the admin menu and its padding.


Well, I'm probably overthinking this beyond the scope of the pull request.

As an aside.. Working on the frontend CSS is so different from the JS/TypeScript internals and backend. A difficulty I'm experiencing is how hard it is to take a series of screenshots at various widths, both with and without the admin top bar.

Since the build script's dev feature is not supported on Firefox yet ("import module from service worker"), I've been using Chromium. From an article on How to take a screenshot in Chrome.

Chrome's native screenshot tools are incredibly easy to use, but also incredibly convoluted to access. The average user may struggle to find where they're located or even how they can call them up. They're actually housed in Chrome's developer tools panel, so users will need to search for them.

..Once you’ve called up the developer tools panel, next you’ll open up the command menu. To do this, you’ll press Control-Shift-P in Windows or Linux, or Command-Shift-P on a Mac.

Now type “screenshot” into the command menu, which will present you four screenshot options.

For every screenshot (every responsive breakpoint), I had to adjust the browser window width/height; press a 3-key combination; type "shot"; press the down arrow 3 times to select "Capture screenshot", and press enter. Then later rename the files to have more meaningful titles.

It would be nice to run a command in the terminal to do all this automatically. I suppose Playwright is the tool to use. (I don't mean for Playground necessarily, I just thought it seems like a common need for any project with a web frontend.)

@adamziel
Copy link
Collaborator

adamziel commented May 30, 2023

The CSS changes look great, thank you @eliot-akira! Let's merge 🎉 🎉 🎉

I wonder if they should be made the same color, so when the user is logged in, the toolbar looks like an extension of the admin menu. On the other hand, maybe the toolbar should have a more distinct background color to clarify that it's a different component.

Good question! They were intended to have the same color, I'm not sure where the subtle difference came from. In any case, I don't have any preference here and would be happy with either.

That made me think about accessibility (...) ARIA labels

Good call, ARIA labels are always worth adding and it would be great to make Playground more accessible. Let's track this separately as we'd need to get an idea of the largest Playground's shortcomings when it's experienced through assistive technologies. I have never tried it!

As an aside.. Working on the frontend CSS is so different from the JS/TypeScript internals and backend. A difficulty I'm experiencing is how hard it is to take a series of screenshots at various widths, both with and without the admin top bar.

I like to use the standard Mac partial capture tool (cmd + shift + 4) to grab the screenshots of the mobile viewport provided by Chrome dev tools. They contain some extra stuff but that's okay – the CSS changes are illustrated well:

CleanShot 2023-05-30 at 10 51 23@2x

It's still a manual process, though, and would still be nice to automate.

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.

Mobile looks broken
2 participants