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

Try optimizing the default column width #7112

Merged
merged 1 commit into from
Jun 6, 2018
Merged

Conversation

jasmussen
Copy link
Contributor

This PR does a few things.

  • Primarily it changes the default column width. It used to be 608, now it's 580px. 580px is about the width of 70 characters, which is widely suggested as the maximum amount of characters per column for readibility. Don't worry, you can still customize this width with an editor style, we've recently made that easier to do: https://wordpress.org/gutenberg/handbook/extensibility/theme-support/#changing-the-width-of-the-editor
  • It retires a text-editor width variable, and uses the same content width variable, so the two editors are the same widths.
  • It removes a few ToDo's.

I'm open to suggestions as to the width of the main column — this is a suggestion for now. But it would be nice to choose a nice and round number other than 608, and apply it to both the visual and the text editor.

Screenshots, before:

screen shot 2018-06-04 at 08 59 23

screen shot 2018-06-04 at 08 59 54

Screenshots, after:

screen shot 2018-06-04 at 09 21 04

screen shot 2018-06-04 at 09 21 07

This PR does a few things.

- Primarily it changes the _default_ column width. It used to be 608, now it's 580px. 580px is about the width of 70 characters, which is widely suggested as the maximum amount of characters per column for readibility. Don't worry, you can still customize this width with an editor style, we've recently made that easier to do: https://wordpress.org/gutenberg/handbook/extensibility/theme-support/#changing-the-width-of-the-editor
- It retires a text-editor width variable, and uses the same content width variable, so the two editors are the same widths.
- It removes a few ToDo's.
@jasmussen jasmussen added the Needs Design Feedback Needs general design feedback. label Jun 4, 2018
@jasmussen jasmussen self-assigned this Jun 4, 2018
@jasmussen jasmussen requested a review from a team June 4, 2018 07:29
Copy link
Member

@tofumatt tofumatt left a comment

Choose a reason for hiding this comment

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

Looks nicer and makes sense to me!

more-red-than-green

@jasmussen
Copy link
Contributor Author

😂

Thanks Matt, appreciated!

I'm going to await some thoughts on the specific width as well!

@GlennMartin1
Copy link

Seems about right. https://baymard.com/blog/line-length-readability

@karmatosed karmatosed self-requested a review June 4, 2018 14:07
Copy link
Member

@karmatosed karmatosed left a comment

Choose a reason for hiding this comment

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

The content width change makes sense to me 👍

@jasmussen jasmussen added this to the 3.1 milestone Jun 6, 2018
@jasmussen jasmussen merged commit 76eaac0 into master Jun 6, 2018
@jasmussen jasmussen deleted the try/smaller-default-width branch June 6, 2018 08:41
@azaozz
Copy link
Contributor

azaozz commented Jun 6, 2018

Seems the default max-width will also have to be changed in https://github.com/WordPress/gutenberg/blob/master/editor/store/defaults.js#L62.

Unless I'm missing something https://wordpress.org/gutenberg/handbook/extensibility/theme-support/#changing-the-width-of-the-editor seems to be missing the fact that maxWidth has to be set in JS too.

Also, allowing plugins and themes to set arbitrary max-width for the editor will need more UI tests. What happens to the UI when it is 380px wide? What about 870px or 1024px? :)

Generally thinking that allowing plugins and themes to (drastically) change the editor appearance is not a good idea. Gutenberg is not WYSIWYG, it doesn't have to be. Letting themes and plugins change some of the styles is fine, allowing "arbitrary CSS" -- not so much. Background: about 1/4 of the problems reported in support forums with the classic editor are caused by editor-style.css :)

Edit: actually this is probably worth a new issue.

jasmussen pushed a commit that referenced this pull request Jun 7, 2018
Addressed feedback in #7112 (comment). Updates the default column width to match that of the JS constant.
@jasmussen
Copy link
Contributor Author

Seems the default max-width will also have to be changed in https://github.com/WordPress/gutenberg/blob/master/editor/store/defaults.js#L62.

Thank you! PR in #7198.

Unless I'm missing something https://wordpress.org/gutenberg/handbook/extensibility/theme-support/#changing-the-width-of-the-editor seems to be missing the fact that maxWidth has to be set in JS too.

I'm not sure how to do that — can you help me here? Thanks.

Generally thinking that allowing plugins and themes to (drastically) change the editor appearance is not a good idea. Gutenberg is not WYSIWYG, it doesn't have to be. Letting themes and plugins change some of the styles is fine, allowing "arbitrary CSS" -- not so much. Background: about 1/4 of the problems reported in support forums with the classic editor are caused by editor-style.css :)

I disagree that allowing themers to style the editor is a bad idea, especially in an editor whose main concept is to allow greater visual freedom, the disconnect between the admin and the end result can be jarring, and for that reason, direct manipulation and WYSIWYG have been part of the Gutenberg vision for more than a year. Of course we need to ensure a good, accessible, basic layout style. We can even consider a user option to turn off an theme editor style if people prefer the default Gutenberg style, just like how web browsers let you override website styles. That could be worth a new ticket.

@gziolo
Copy link
Member

gziolo commented Jun 7, 2018

Unless I'm missing something https://wordpress.org/gutenberg/handbook/extensibility/theme-support/#changing-the-width-of-the-editor seems to be missing the fact that maxWidth has to be set in JS too.

I'm not sure how to do that — can you help me here? Thanks.

Should we expose another filter on PHP side to make it customizable, similar to what we do with colors?

@jasmussen
Copy link
Contributor Author

I'm not sure what the variable does. I'm guessing it is used when inserting images to preset dimensions perhaps. In any case, if it's an important variable for themers, we should ideally make it as easy as possible for them to customize.

@azaozz
Copy link
Contributor

azaozz commented Jun 7, 2018

Uh, sorry, I opened #7180 for using either CSS or the maxWidth value set in JS but forgot to link it here. Yes, this is needed mostly for properly displaying images and calculating width as percentage, but may be useful in other cases too (embeds?).

I disagree that allowing themers to style the editor is a bad idea

I don't mean we should completely disallow that, but imho it would be much better to have some strict restrictions on what can be added/changed. There are several reasons:

  1. This will "lock" us in a heavy back-compat mode. Changing the DOM inside the editor even a little bit may cause style breakages (from wrong colors/unreadable text to "invisible" elements, unworkable controls, etc.).
  2. The blocks in Gutenberg have "mixed" content, i.e. the controls and the editable HTML are next to each other and in many wrappers. Because of this the HTML inside a block looks nothing like it would look on the front-end. There are a lot of "HTML violations", invalid nested elements, etc. Because of these discrepancies styling inside the editor is quite different from styling the same content on the front-end. These differences also mean the back-compat "locking" will be a lot worse than currently in the classic editor (where the HTML inside the editor is exactly the same as on the front-end).
  3. Because of 2, the theme added styling may "bleed" over to other blocks and make them unusable.
  4. Tons and tons of support requests related to a plugin or theme unexpectedly changing some style inside the editor. Example: "Why is my table without borders? I can't see where the cells begin and end..?". Result: blame the editor! :)

jasmussen added a commit that referenced this pull request Jun 11, 2018
Addressed feedback in #7112 (comment). Updates the default column width to match that of the JS constant.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Design Feedback Needs general design feedback.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants