WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.49k stars 4.18k forks source link

WP5.4 Beta1 (& 2) removed classes that are in use causing theme layout to break #20307

Closed maddisondesigns closed 4 years ago

maddisondesigns commented 4 years ago

Describe the bug In WP5.4, numerous classes that were used within the Editor screens, have been removed/changed, which breaks the layout of my theme within the editor. These classes have been in use even before Gutenberg was introduced into Core in 5.0. Why are these breaking changes being introduced now?

As an example, currently the classes for a Paragraph block are wp-block editor-block-list__block block-editor-block-list__block.

With 5.4, these classes change to wp-block block-editor-block-list__block has-selected-ui.

Not only does it affect the width of all the blocks in the editor, but also things like fonts, and alignments such as Full and Width widths.

The width of every block has been affected, including Paragraph Blocks. I can also see that alignments such as wide-width and full-width have been affected. This is just what I've noticed so far. I haven't had time to check every individual block to see what else has been broken.

I can see that you've also altered classes in the Metabox area as well, which also affects some styles that are currently in use.

Expected behaviour Core shouldn't be breaking backwards compatibility. If you need to introduce new classes then do so, but either don't remove the old classes, or at the very least, give developers some time to be able to update their theme/plugins, before removing them, instead of just immediately breaking them. These sort of breaking changes need to be properly documented for Theme/Plugin developers (and no, mentioning it somewhere within some Github issue is not documentation).

Desktop (please complete the following information):

talldan commented 4 years ago

@maddisondesigns Just including some links for your reference.

There's some information about Backward Compatibility here: https://github.com/WordPress/gutenberg/tree/master/docs/designers-developers/developers/backward-compatibility#backward-compatibility

For the classname you mentioned, there was a dev note about renaming the class names here: https://make.wordpress.org/core/2019/04/09/the-block-editor-javascript-module-in-5-2/

And the pull request that more recently removed the editor class names is here - https://github.com/WordPress/gutenberg/pull/19050

maddisondesigns commented 4 years ago

@talldan Thanks for those links. That DevNote specifically says:

The old class names have been retained to minimize any backward compatibility concern, but the CSS styles are now targeting the new class names

As mentioned above, those old class names haven't been retained, so they need to be re-added.

maddisondesigns commented 4 years ago

Also that DevNote only lists one example of a Class that has changed:

For example, the editor-inserter__toggle class name is now renamed block-editor-inserter__toggle.

Every class that has changed should be listed otherwise how do we know what's actually changed?

talldan commented 4 years ago

@maddisondesigns The intention was to give developers enough time to update code by supporting both class names (similar to what you mention in the issue description), the advice in the dev note was that any usage should be renamed from editor to block-editor and preferably not used at all.

I understand that it might not have been clear enough which class names were affected (and also that it's hard to keep up with changes). I think it's a good opportunity to understand how dev notes can be improved. A counter-argument could be that listing every class name would have made the dev note incredibly long and harder to read, but maybe linking to a full list elsewhere would be an option.

cc @aduth, @youknowriad for additional thoughts.

maddisondesigns commented 4 years ago

@talldan That's woefully insufficient. There's a class in use in the editor called .editor-post-title__input. Based on the assumption that "any usage should be renamed from editor to block-editor", then this class should've also been changed to .block-editor-post-title__input, but it hasn't.

There is soooo much room for ambiguity here, it's ridiculous. There needs to be a complete list of every single Class/ID that has changed names or been removed, every single ID that has changed to a Class or been removed, etc...

This needs to be updated, every single time something like this in Core changes. How else are Theme/Plugin developers supposed to keep track of these things?? And it needs to be documented in official documentation, not in some DevNote blog post where it's impossible to find unless you specifically know what you're looking for. It's not good enough for Core Devs to simply do development and not update documentation at the same time.

jorgefilipecosta commented 4 years ago

There is an ongoing effort to simplify the block dom and make it more similar to the frontend. Multiple wrappers div's classes etc have been removed. As documented in https://github.com/WordPress/gutenberg/tree/master/docs/designers-developers/developers/backward-compatibility#backward-compatibility classes and the dom structure are not part of the public API and they are subject to change.

Until now, the way these changes are communicated is using a dev note. We plan on publishing a dev specific to classes/markup changes.

Dev notes are being worked on at https://github.com/WordPress/gutenberg/issues/20185. More specifically the dev notes related to this issue are https://github.com/WordPress/gutenberg/issues/20185#issuecomment-585860603, https://github.com/WordPress/gutenberg/issues/20185#issuecomment-585253689.

If anyone was relying on a class that existed before and whose change is not documented in the two previous linked dev notes please let us know and we will try to update the dev note accordingly. Thank you in advance for any help/insights that may be provided.

maddisondesigns commented 4 years ago

@jorgefilipecosta So, because "classes and the dom structure are not part of the public API", you're happy to just break backwards compatibility!? Really?

There are rare occasions where breaking backward compatibility is unavoidable and in those cases the breakage...

From what I've seen in my case, at least so far, is that I haven't been affected by your removal of div elements, so this should be the easiest thing to resolve. It's far from "unavoidable". All you need to do is put back the classes you removed. Classes that have been in use for at least a couple of years now. It won't break your changes and it will keep backwards compatibility so that sites don't break. This needs to be done, at least for a couple of major versions, to give theme and plugin developers time to update their code, and to give thousands of sites who are using those themes and plugins, plenty of time to update.

Putting items like these in DevNotes (either in Github or on the dot org site), is basically pointless for anybody who doesn't already know about these changes. Are you expecting us to read every single Github issue and every DevNote post on dot org, so that we can keep track of these changes? That's just ridiculous! How else are we going to know what's changed unless you add this to some official documentation. A single point where we can regularly check for what's been updated/changed. Please tell me how I would've found out about this change, unless I read through every single DevNote. Even if I had of read that DevNote (mentioned above) on Make WordPress, it was clearly insufficient to make sense of as I already explained above. Instead, the only way I found out about this is from installing the beta version and debugging the issue myself.

If anyone was relying on a class that existed before and whose change is not documented in the two previous linked dev notes please let us know and we will try to update the dev note accordingly.

Regardless of whether it's mentioned in a DevNote, any class that was removed, should be re-added. This isn't just a simple matter of developers updating their themes/plugins. If someone is using these theme and plugins from dot org, and they don't update, then it makes no difference whether the actual theme/plugin has been updated or not, it's still going to break their site once 5.4 rolls out if these classes aren't reinstated.

At the very least, these are the Classes/IDs that I've found so far that have affected myself, and that are causing layout breakages and need to be re-added.

Classes within Block Editor section that need to be re-added .editor-block-list__block .editor-default-block-appender

IDs within Metabox section that need to be re-added #poststuff

maddisondesigns commented 4 years ago

This is still broken in Beta 3

aduth commented 4 years ago

There was a transition period, since the time these class names were deprecated in WordPress 5.2. From the previously-mentioned devnote:

Ideally, you should rely on components in your own code instead of the internal class names used in the WordPress admin. But if you do use those classes, make sure to rename them accordingly.

The post included a full set of affected components:

Click to expand
Autocomplete
AlignmentToolbar
BlockAlignmentToolbar
BlockControls
BlockEdit
BlockEditorKeyboardShortcuts
BlockFormatControls
BlockIcon
BlockInspector
BlockList
BlockMover
BlockNavigationDropdown
BlockSelectionClearer
BlockSettingsMenu
BlockTitle
BlockToolbar
ColorPalette
ContrastChecker
CopyHandler
createCustomColorsHOC
DefaultBlockAppender
FontSizePicker
getColorClassName
getColorObjectByAttributeValues
getColorObjectByColorValue
getFontSize
getFontSizeClass
Inserter
InnerBlocks
InspectorAdvancedControls
InspectorControls
PanelColorSettings
PlainText
RichText
RichTextShortcut
RichTextToolbarButton
RichTextInserterItem
UnstableRichTextInputEvent
MediaPlaceholder
MediaUpload
MediaUploadCheck
MultiBlocksSwitcher
MultiSelectScrollIntoView
NavigableToolbar
ObserveTyping
PreserveScrollInReorder
SkipToSelectedBlock
URLInput
URLInputButton
URLPopover
Warning
WritingFlow
withColorContext
withColors
withFontSizes

Unlike the components listed here, the internal class names are neither part of the public API nor documented. However, following CSS naming guidelines, they can be mapped to corresponding class names matching or starting with:

Click to expand
editor-autocomplete
editor-autocomplete__*
editor-alignment-toolbar
editor-alignment-toolbar__*
editor-block-alignment-toolbar
editor-block-alignment-toolbar__*
editor-block-controls
editor-block-controls__*
editor-block-edit
editor-block-edit__*
editor-block-editor-keyboard-shortcuts
editor-block-editor-keyboard-shortcuts__*
editor-block-format-controls
editor-block-format-controls__*
editor-block-icon
editor-block-icon__*
editor-block-inspector
editor-block-inspector__*
editor-block-list
editor-block-list__*
editor-block-mover
editor-block-mover__*
editor-block-navigation-dropdown
editor-block-navigation-dropdown__*
editor-block-selection-clearer
editor-block-selection-clearer__*
editor-block-settings-menu
editor-block-settings-menu__*
editor-block-title
editor-block-title__*
editor-block-toolbar
editor-block-toolbar__*
editor-color-palette
editor-color-palette__*
editor-contrast-checker
editor-contrast-checker__*
editor-copy-handler
editor-copy-handler__*
editor-default-block-appender
editor-default-block-appender__*
editor-font-size-picker
editor-font-size-picker__*
editor-inserter
editor-inserter__*
editor-inner-blocks
editor-inner-blocks__*
editor-inspector-advanced-controls
editor-inspector-advanced-controls__*
editor-inspector-controls
editor-inspector-controls__*
editor-panel-color-settings
editor-panel-color-settings__*
editor-plain-text
editor-plain-text__*
editor-rich-text
editor-rich-text__*
editor-rich-text-shortcut
editor-rich-text-shortcut__*
editor-rich-text-toolbar-button
editor-rich-text-toolbar-button__*
editor-rich-text-inserter-item
editor-rich-text-inserter-item__*
editor-unstable-rich-text-input-event
editor-unstable-rich-text-input-event__*
editor-media-placeholder
editor-media-placeholder__*
editor-media-upload
editor-media-upload__*
editor-media-upload-check
editor-media-upload-check__*
editor-multi-blocks-switcher
editor-multi-blocks-switcher__*
editor-multi-select-scroll-into-view
editor-multi-select-scroll-into-view__*
editor-navigable-toolbar
editor-navigable-toolbar__*
editor-observe-typing
editor-observe-typing__*
editor-preserve-scroll-in-reorder
editor-preserve-scroll-in-reorder__*
editor-skip-to-selected-block
editor-skip-to-selected-block__*
editor-url-input
editor-url-input__*
editor-url-input-button
editor-url-input-button__*
editor-url-popover
editor-url-popover__*
editor-warning
editor-warning__*
editor-writing-flow
editor-writing-flow__*

On the point of "official documentation", release devnotes such as these are intended to serve as the point of communication for core changes which are expected to be impactful to plugin and theme developers (reference).

https://make.wordpress.org/core/tag/dev-notes/

These are typically marked at the point of introduction as "Needs Dev Note", which can also serve as a point of reference for future impactful changes:

Regarding #poststuff, my understanding is this is discussed in Trac#46964. Based on recent conversation, it appears the change has since been reverted.

maddisondesigns commented 4 years ago

@aduth Thanks for those details.

Ideally, you should rely on components in your own code instead of the internal class names used in the WordPress admin. But if you do use those classes, make sure to rename them accordingly.

When you're developing a theme and you're trying to style the Block Editor in the Dashboard, this is literally impossible. There's no way to style it without relying on the internal class names used in the WordPress admin.

While those list of affected components may be useful to someone working on Gutenberg itself, they're not that useful, or meaningful, to someone who's simply developing themes or plugins. It also makes a DevNote statement like "For example, the editor-insertertoggle class name is now renamed block-editor-insertertoggle" pretty much meaningless. As I mentioned above, if that is supposed to infer that any class usage should be renamed from editor to block-editor, then why wasn't the Title block also change from .editor-post-title__input to .block-editor-post-title__input? That seems very inconsistent. It's still a block, like every other block, and based on the class name starting with 'editor-', it's presumably part of the block-editor module as well.

As I mentioned above, expecting plugin/theme developers to read every single DevNote, is unreasonable and just ridiculous, especially considering that the majority of info in these is going to be irrelevant, and also mostly meaningless, unless you've actually worked on the Gutenberg codebase itself.

mcsf commented 4 years ago

@maddisondesigns: Doing my best to refrain from debate, I'll just stress that dev notes are indeed important reading material for any developer building plugins or themes.

That said, I invite you to try this small plugin: https://github.com/mcsf/find-deprecated-css-in-wp52. I wrote it in the hopes that it can make developers' task of updating their products easier. Any feedback on the tool is welcome.