Skip to content
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

Polish header for mobile and more languages #5329

Merged
merged 5 commits into from
Mar 2, 2018
Merged

Conversation

jasmussen
Copy link
Contributor

Fixes #4231.

This PR enhances the editor-bar for mobile, and has been tested with German on an iPhone 5 screen.

It does a few things to free up space and be smart about the mobile UI:

  • It tweaks paddings, margins widths
  • It hides the "Preview" and "Publish" buttons when the "Switch to Draft" button is showing — publish is disabled at that point anyway
  • It adds an icon for the "Saving" state (CSS animated with a little pulse), and as such we can make the save indicator be icon-only on mobile.

Many further improvements can, and should, be made to the mobile UI. But this addresses the things that are directly broken, and aside from general future mobile polish, I would encourage opening separate tickets for mobile header space issues.

Screenshots:

screen shot 2018-03-01 at 13 24 20

screen shot 2018-03-01 at 13 37 15

english

Joen Asmussen added 4 commits March 1, 2018 13:01
@jasmussen jasmussen self-assigned this Mar 1, 2018
@aduth aduth added Design General Interface Parts of the UI which don't fall neatly under other labels. Internationalization (i18n) Issues or PRs related to internationalization efforts Mobile Web Viewport sizes for mobile and tablet devices labels Mar 1, 2018
@aduth aduth added this to the 2.3 milestone Mar 1, 2018
@youknowriad
Copy link
Contributor

This is looking good, I pushed a small commit to fix the unit tests.

The issue I have with the recent updates to the top is the spacing : The space between the publish button and the cog button in mobile is different.

screen shot 2018-03-02 at 09 51 37

But it's more visible for me in desktop (which I think is not related to this PR). In that case, the space between "Publish" and "Preview" is too small compared to the space between "Publish" and the "cog" button.

screen shot 2018-03-02 at 09 50 47

Looks good otherwise.

@youknowriad
Copy link
Contributor

I think we should start using CSS Grids more in our stylesheets to fix all these alignments and spacing issues at the parent level which is less subject to errors and updates.

display: grid;
grid-auto-flow: column;
grid-gap: 5px;
align-content: center;
align-items: center;

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@jasmussen
Copy link
Contributor Author

jasmussen commented Mar 2, 2018

But it's more visible for me in desktop (which I think is not related to this PR). In that case, the space between "Publish" and "Preview" is too small compared to the space between "Publish" and the "cog" button.

Yep, that's intentional insofar as I've been trying to free up space to fit the most important buttons in multiple languages. German gets very long.

I think improvement can certainly be made, and I like your grid work, but there's not much space we can use if we need to be able to show both preview and publish in german on an iPhone 5. So for now I think it's worth addressing separately, perhaps as part of #5343.

Thanks for the review, merging (when the tests pass)!

@jasmussen jasmussen merged commit 9c164f3 into master Mar 2, 2018
@jasmussen jasmussen deleted the polish/mobile-header branch March 2, 2018 09:03
@youknowriad
Copy link
Contributor

Yep, that's intentional insofar as I've been trying to free up space to fit the most important buttons in multiple languages. German gets very long.

I don't care if it's the small spacing we choose between the buttons, I just prefer if it's consistent (just use a smaller grid-gap

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
General Interface Parts of the UI which don't fall neatly under other labels. Internationalization (i18n) Issues or PRs related to internationalization efforts Mobile Web Viewport sizes for mobile and tablet devices
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants