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

Consider removing either the JS maxWidth or the ability to set max-width from CSS #7180

Open
azaozz opened this issue Jun 6, 2018 · 6 comments
Labels
Needs Decision Needs a decision to be actionable or relevant Needs Dev Ready for, and needs developer efforts [Type] Enhancement A suggestion for improvement.

Comments

@azaozz
Copy link
Contributor

azaozz commented Jun 6, 2018

The editor width can be set by themes and plugins from CSS. At the same time there is a https://github.com/WordPress/gutenberg/blob/master/editor/store/defaults.js#L62 maxWidth JS setting. Thinking there should be either one or the other, but not both.

If we are keeping the CSS max-width, it would be great to have a property (maxWidth, editorWidth...) set in JS, but it will have to be set after the editor loads. Alternatively can disallow (always override) the CSS max-width in favor of the JS maxWidth. Plugins and themes can use that instead when they want to change it.

@azaozz azaozz added the Needs Decision Needs a decision to be actionable or relevant label Jun 6, 2018
@azaozz azaozz changed the title Consider removing either the LS maxWidth or the ability to set max-width from CSS Consider removing either the JS maxWidth or the ability to set max-width from CSS Jun 6, 2018
@azaozz azaozz added the Needs Dev Ready for, and needs developer efforts label Jun 7, 2018
@azaozz
Copy link
Contributor Author

azaozz commented Jun 7, 2018

Thinking a bit more about this, we should probably drop the JS var and replace it with a small utility function returning the current editor width (that would run on render) and be globally accessible. Having a hard-coded width would fail on small screen devices when the responsive styling kicks in.

@lzs
Copy link

lzs commented Aug 25, 2018

I wonder if this is still being worked on? I see some elements (such as a DIV around a IMG within a wp-block-image) that is styled directly with "max-width: 580px", presumably from $maxWidth.

@gziolo
Copy link
Member

gziolo commented May 23, 2019

@jasmussen - is it something we still need?

@jasmussen
Copy link
Contributor

I'm unsure. Maybe Andrew is best to respond?

@azaozz
Copy link
Contributor Author

azaozz commented May 23, 2019

The maxWidth "constant" is still there https://github.com/WordPress/gutenberg/blob/master/packages/block-editor/src/store/defaults.js#L124, and is sometimes "incorrect" :) (actual width is changed by theme/plugin CSS). Not sure why it exists.

If it is to be used for images, it needs to be correct, or it just messes things up. Perhaps a global utility function to get that from the current dom (i.e. what the user sees at the moment) would be better.

On the other hand, fixing this would be best done when we know how themes are going to use context for images on the front-end, or if they will need something like that at all. (Ideally on the front-end all images will always have width and height attributes or we end up with jumping pages..).

@kadamwhite
Copy link
Contributor

Cross-referencing to #6652 based on @azaozz' latest comment, above.

maxWidth also appears to be part of the issue behind #17795

@jordesign jordesign added the [Type] Enhancement A suggestion for improvement. label Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Decision Needs a decision to be actionable or relevant Needs Dev Ready for, and needs developer efforts [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

6 participants