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

Pre-publish panel panels lack spaces between titles and labels #7842

Closed
sarahmonster opened this issue Jul 9, 2018 · 4 comments
Closed

Pre-publish panel panels lack spaces between titles and labels #7842

sarahmonster opened this issue Jul 9, 2018 · 4 comments
Assignees
Labels
[Type] Enhancement A suggestion for improvement.
Milestone

Comments

@sarahmonster
Copy link
Member

Issue Overview

There isn't any space between the titles and the labels of panels inside the pre-publish panel:
screenshot 2018-07-09 20 06 39

Steps to Reproduce (for bugs)

  1. Create a new post using Gutenberg.
  2. Press the "Publish" button.
  3. Notice lack of spaces!

Looks as though the translation strings here are written with the intention of there being a single space here (<PanelBody title={ [__( 'Visibility: ' )] }>), but that trailing space is removed when the text string is run through the translation function. Text strings aren't supposed to use trailing whitespace, so a little bit of CSS padding is probably a better solution here.

Sort of related (if pretty tangentially): #7426

@sarahmonster sarahmonster added [Type] Enhancement A suggestion for improvement. [Type] Copy Issues or PRs that need copy editing assistance labels Jul 9, 2018
@sarahmonster sarahmonster self-assigned this Jul 9, 2018
@danielbachhuber
Copy link
Member

👍 I've been noticing this too but never got around to create a ticket for it.

@chrisvanpatten
Copy link
Contributor

Only concern I have about CSS padding is what happens if you select and copy/paste that text — presumably there's no space being copied? Also, what's the impact on screen readers if the space isn't in the markup?

@tofumatt
Copy link
Member

Sounds like, from the issue description, we trim whitespace in the strings so it doesn't matter, but that isn't true. In master, I see:

screenshot 2018-07-10 09 42 28

(Note the space after "Visibility".)

In this case the whitespace is part of the translation and should stay, but we should also have some padding.

@sarahmonster
Copy link
Member Author

These are clickable titles that open up panels, so I'm not sure there'd be a way of copying and pasting the text. (This is me trying!)

2018-07-09 23 24 43

The screenreader issue is a valid concern, although I wasn't able to find solid documentation one way or another to indicate how a screenreader would handle a case like this. Would the colon and the span be enough to force a space in the way it's read?

I could leave the whitespace in the title, but that doesn't seem to be best practise. From the i18n documentation:

Do not leave leading or trailing whitespace in a translatable phrase.

I suspect this is because GlotPress may not display leading/trailing whitespace in an obvious way, which means the string would be likely to be translated without the trailing whitespace—which means we'd have the same issue with other languages.

@tofumatt tofumatt added this to the 3.3 milestone Jul 10, 2018
@tofumatt tofumatt removed the [Type] Copy Issues or PRs that need copy editing assistance label Jul 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

4 participants