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

Gutenberg frontend style.css has admin CSS #11668

Closed
jdevalk opened this issue Nov 9, 2018 · 12 comments · Fixed by #11752
Closed

Gutenberg frontend style.css has admin CSS #11668

jdevalk opened this issue Nov 9, 2018 · 12 comments · Fixed by #11752
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. [Type] Code Quality Issues or PRs that relate to code quality [Type] Enhancement A suggestion for improvement.

Comments

@jdevalk
Copy link
Contributor

jdevalk commented Nov 9, 2018

Describe the bug
On the frontend, every site with Gutenberg gets block-library/style.css loaded. That file contains CSS that is meant for the backend, like this:

@keyframes fade-in{
    0%{
        opacity:0
    }
    to{
        opacity:1
    }
}
@keyframes editor_region_focus{
    0%{
        box-shadow:inset 0 0 0 0 #33b3db
    }
    to{
        box-shadow:inset 0 0 0 4px #33b3db
    }
}
@keyframes rotation{
    0%{
        transform:rotate(0deg)
    }
    to{
        transform:rotate(1turn)
    }
}
@keyframes modal-appear{
    0%{
        margin-top:32px
    }
    to{
        margin-top:0
    }
}

To Reproduce
Steps to reproduce the behavior:

  1. Go to https://joost.blog
  2. View the source, find block-library/style.css
  3. See the above at the top. See that it's not used in that CSS file anywhere.

Expected behavior
This should not be included in that CSS file.

Desktop (please complete the following information):

  • OS: any
  • Browser any
  • Version WP 5.0 beta
@jdevalk
Copy link
Contributor Author

jdevalk commented Nov 9, 2018

Another bit that clearly doesn't belong in here:

.editor-block-list__layout .reusable-block-edit-panel{
    align-items:center;
    background:#f8f9f9;
    color:#555d66;
    display:flex;
    flex-wrap:wrap;
    font-family:-apple-system,BlinkMacSystemFont,Segoe UI,Roboto,Oxygen-Sans,Ubuntu,Cantarell,Helvetica Neue,sans-serif;
    font-size:13px;
    position:relative;
    top:-14px;
    margin:0 -14px -14px;
    padding:8px 14px
}
.editor-block-list__layout .editor-block-list__layout .reusable-block-edit-panel{
    margin:0 -14px;
    padding:8px 14px
}
.editor-block-list__layout .reusable-block-edit-panel .reusable-block-edit-panel__spinner{
    margin:0 5px
}
.editor-block-list__layout .reusable-block-edit-panel .reusable-block-edit-panel__info{
    margin-right:auto
}
.editor-block-list__layout .reusable-block-edit-panel .reusable-block-edit-panel__label{
    margin-right:8px;
    white-space:nowrap;
    font-weight:600
}
.editor-block-list__layout .reusable-block-edit-panel .reusable-block-edit-panel__title{
    flex:1 1 100%;
    font-size:14px;
    height:30px;
    margin:4px 0 8px
}
.editor-block-list__layout .reusable-block-edit-panel .components-button.reusable-block-edit-panel__button{
    flex-shrink:0
}
@media (min-width:960px){
    .editor-block-list__layout .reusable-block-edit-panel{
        flex-wrap:nowrap
    }
    .editor-block-list__layout .reusable-block-edit-panel .reusable-block-edit-panel__title{
        margin:0
    }
    .editor-block-list__layout .reusable-block-edit-panel .components-button.reusable-block-edit-panel__button{
        margin:0 0 0 5px
    }
}
.editor-block-list__layout .reusable-block-indicator{
    background:#fff;
    border-left:1px dashed #e2e4e7;
    color:#555d66;
    border-bottom:1px dashed #e2e4e7;
    top:-14px;
    height:30px;
    padding:4px;
    position:absolute;
    z-index:1;
    width:30px;
    right:-14px
}

All of this seems editor CSS to me.

@jdevalk
Copy link
Contributor Author

jdevalk commented Nov 9, 2018

Note that the above examples are from minified files that I "unminified", code might not look like that in the source :)

@youknowriad
Copy link
Contributor

Agreed with the issues mentioned here: (about the animations, the reusable one need further investigation)

We have an _animations.scss file that is meant to be a "mixin" the issue is that CSS animtations can't be defined as mixins properly so they end up duplicated automatically in all stylesheets.

I suggest removing this file by doing something like:

  • If an animation is used only once in Gutenberg, define it in the stylesheet actually using it and give a very specific name
  • If an animation is used in multiple places, we can keep this generic animations file but not import it implicitely (like other variables and mixins files) and only import it explicitely when a stylesheet (package) need it. (In addition to making these animations names more specific).

@designsimply designsimply added [Type] Enhancement A suggestion for improvement. [Type] Code Quality Issues or PRs that relate to code quality labels Nov 11, 2018
jasmussen pushed a commit that referenced this issue Nov 12, 2018
This PR maybe fixes #11673, maybe fixes #11668. At least, this is the intention. It does two things:

- It changes the naming scheme for all animations. Instead of having a mix of underscores and dashes for word separation, it uses BEM (to the best of my ability) to add consistency and a new naming convention for all animations.
- It adds a prefix to all animations, indicating they are for the editor. Though perhaps we should have a more _admin_ specific prefix given these might one day be used outside the admin?
- It moves all keyframe animations out of the mixins file, and into the edit-post style. This is because keyframe definitions don't work well with mixins and are just duplicated.

This PR has been tested for all the animations I could find that were using the ones defined. But please give me a sanity check.

By the way I noticed the following two animations do not appear to be used:

- editor-animation__animate-fade
- editor-animation__move-background

Should we remove those? Or can anyone recall where they were intended to be used?
@m-e-h
Copy link
Member

m-e-h commented Nov 12, 2018

A couple more I think:

.is-selected .wp-block-gallery .blocks-gallery-image:nth-last-child(2),
.is-selected .wp-block-gallery .blocks-gallery-item:nth-last-child(2),
.is-typing .wp-block-gallery .blocks-gallery-image:nth-last-child(2),
.is-typing .wp-block-gallery .blocks-gallery-item:nth-last-child(2),

jasmussen pushed a commit that referenced this issue Nov 13, 2018
This PR maybe fixes #11673, maybe fixes #11668. At least, this is the intention. It does two things:

- It changes the naming scheme for all animations. Instead of having a mix of underscores and dashes for word separation, it uses BEM (to the best of my ability) to add consistency and a new naming convention for all animations.
- It adds a prefix to all animations, indicating they are for the editor. Though perhaps we should have a more _admin_ specific prefix given these might one day be used outside the admin?
- It moves all keyframe animations out of the mixins file, and into the edit-post style. This is because keyframe definitions don't work well with mixins and are just duplicated.

This PR has been tested for all the animations I could find that were using the ones defined. But please give me a sanity check.

By the way I noticed the following two animations do not appear to be used:

- editor-animation__animate-fade
- editor-animation__move-background

Should we remove those? Or can anyone recall where they were intended to be used?
jasmussen pushed a commit that referenced this issue Nov 13, 2018
Gallery related, in #11668 (comment).
jasmussen added a commit that referenced this issue Nov 14, 2018
* Code Quality: Improve prefix, better scope, for animations

This PR maybe fixes #11673, maybe fixes #11668. At least, this is the intention. It does two things:

- It changes the naming scheme for all animations. Instead of having a mix of underscores and dashes for word separation, it uses BEM (to the best of my ability) to add consistency and a new naming convention for all animations.
- It adds a prefix to all animations, indicating they are for the editor. Though perhaps we should have a more _admin_ specific prefix given these might one day be used outside the admin?
- It moves all keyframe animations out of the mixins file, and into the edit-post style. This is because keyframe definitions don't work well with mixins and are just duplicated.

This PR has been tested for all the animations I could find that were using the ones defined. But please give me a sanity check.

By the way I noticed the following two animations do not appear to be used:

- editor-animation__animate-fade
- editor-animation__move-background

Should we remove those? Or can anyone recall where they were intended to be used?

* Address feedback.

This retires a few animations, and moves others that are only used once to their respective scopes.

* Address comment.

Gallery related, in #11668 (comment).

* Address feedback.

* Address final point.
@m-e-h
Copy link
Member

m-e-h commented Nov 14, 2018

@jasmussen was this meant to be closed? I think maybe the @mixins and @keyframes may have gotten the original issue, Gutenberg frontend style.css has admin CSS, a little off track.

There are still plenty of editor styles currently in the frontend CSS.

Is there a reason these files are added to the front-end? https://github.com/WordPress/gutenberg/blob/master/packages/block-library/src/style.scss#L2-L3

@m-e-h
Copy link
Member

m-e-h commented Nov 14, 2018

.editor-block-list__block[data-type="core/embed"][data-align="left"]
.editor-block-list__block-edit,
.editor-block-list__block[data-type="core/embed"][data-align="right"]
.editor-block-list__block-edit,

@jdevalk
Copy link
Contributor Author

jdevalk commented Nov 14, 2018

Yeah I agree with @m-e-h. As much as I appreciate the work done in that pull that closed this, this still needs fixing.

@omarreiss omarreiss reopened this Nov 14, 2018
@jasmussen
Copy link
Contributor

Always okay to reopen! Was out for the day when I received this ping.

@m-e-h
Copy link
Member

m-e-h commented Nov 15, 2018

@youknowriad
Copy link
Contributor

Can we clarify what's missing here. I see a number of merged PRs, are there any styles lingering on the frontend still?

@tellthemachines tellthemachines added the CSS Styling Related to editor and front end styles, CSS-specific issues. label Apr 23, 2020
@aristath
Copy link
Member

Can we clarify what's missing here. I see a number of merged PRs, are there any styles lingering on the frontend still?

A few days ago I went through the compiled frontend styles and found some, there's a PR on #24439 for them 👍

@aristath
Copy link
Member

This is no longer an issue, all remaining admin styles were removed from front-facing styles so I'll go ahead and close this ticket. 🎉
If in the future we see more editor styles leak on the frontend we can remove them on a case-by-case basis 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. [Type] Code Quality Issues or PRs that relate to code quality [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants