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

Move the paragraph indent style to the first paragraph of every container #4481

Merged
merged 4 commits into from
Sep 1, 2021

Conversation

scruffian
Copy link
Member

@scruffian scruffian commented Aug 27, 2021

Changes proposed in this Pull Request:

This removes the indented block styles, and replaces them with some CSS to target the first paragraph in each container. The CSS is restricted in scope to only within the post content.

To test, try adding all the block patterns to a post.

Related issue(s):

Fixes #4445

@scruffian scruffian added the [Theme] Skatepark Automatically generated label for Skatepark. label Aug 27, 2021
@scruffian scruffian requested a review from a team August 27, 2021 11:14
@scruffian scruffian self-assigned this Aug 27, 2021
.wp-block-post-author__content,
.wp-block-post-comments,
.wp-block-quote {
Copy link
Member Author

Choose a reason for hiding this comment

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

We might need some more exceptions here.

Copy link
Contributor

Choose a reason for hiding this comment

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

having the post comments block inside wp-block-post-content would be weird, would it even work?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think cover and media & text also need it...

Screen Shot 2021-08-27 at 10 59 31 AM

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Member Author

Choose a reason for hiding this comment

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

having the post comments block inside wp-block-post-content would be weird, would it even work?

It would be weird but it does work. There could be cases where you wanted to encourage comments so you put the form into the post content? If you let people do it, someone will find a use for it!

@MaggieCabrera
Copy link
Contributor

The original issue mentions adding a customizer option to enable/disable this but I don't see how we can translate that to the FSE. Do you think we should add such a setting even if we don't have an FSE counterpart?

@scruffian
Copy link
Member Author

I don't think we should add an option for it.

}

.wp-block-post-content {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add an .is-root-container selector to target the editor as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

I had to use block-editor-writing-flow as is-root-container is also used in the site editor.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not seeing the indent on the editor right now, I don't see that class on the css of this PR either

@MaggieCabrera
Copy link
Contributor

A few cases I found:

Cover block
Screenshot 2021-08-30 at 11 38 45

Media + Text I'm not sure if we should remove it or not. I kinda preferred to have an option to add it as a block style for the user and have it off for this block by default. What do you think?
Screenshot 2021-08-30 at 11 40 38

Pullquote
Screenshot 2021-08-30 at 11 41 39

@melchoyce
Copy link
Contributor

I chatted this over with @kjellr. I'm concerned about how complicated this is becoming — either we need a bunch of CSS rules to overwrite the default behavior so it works for all blocks as expected, or we offload the complexity onto users with a block style. I think we should remove the indentation entirely. It was cool to try, and it's a fun aesthetic choice, but unless we can control it natively in Gutenberg we're going to run into lots of edge cases. The theme's color and duotone support should help it stand out even without the indentation. I appreciate all the work y'all did to try this feature out! Maybe we'll be able to implement it natively in the future.

@MaggieCabrera
Copy link
Contributor

I chatted this over with @kjellr. I'm concerned about how complicated this is becoming — either we need a bunch of CSS rules to overwrite the default behavior so it works for all blocks as expected, or we offload the complexity onto users with a block style. I think we should remove the indentation entirely. It was cool to try, and it's a fun aesthetic choice, but unless we can control it natively in Gutenberg we're going to run into lots of edge cases. The theme's color and duotone support should help it stand out even without the indentation. I appreciate all the work y'all did to try this feature out! Maybe we'll be able to implement it natively in the future.

After Mel's feedback, I removed the custom CSS and left the parts of this PR that were removing the text-indent block styles so we have no indentation anywhere

@melchoyce
Copy link
Contributor

LGTM 👍

Copy link
Contributor

@jffng jffng left a comment

Choose a reason for hiding this comment

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

Removed the classes from the Columns in Container pattern, this LGTM 🚢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Theme] Skatepark Automatically generated label for Skatepark.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Skatepark: Make text-indent the default for first paragraphs
4 participants