Rename default block styles to "Default" by kjellr · Pull Request #13670 · WordPress/gutenberg

kjellr · 2019-02-05T13:49:56Z

Resolves #13660.

This aligns name we're using in the UI with the classname, and also lets us be a little less prescriptive about what exactly the default block styles per theme should look like.

Separator block: Short Line ➡️ Default
Blockquote block: Regular ➡️ Default
Pullquote block: Regular ➡️ Default
Table block: Regular ➡️ Default
Button block: Rounded ➡️ Default

Resolves #13660. This aligns the classname with the actual name we're using in the UI, and also allows for a bit more flexibility for default block styles per theme.

jasmussen · 2019-02-05T13:53:48Z

Love it, thank you! 🤟 from me, I guess it just needs a code sanity check. Pretty sure this doesn't require deprecation handlers.

youknowriad · 2019-02-05T13:55:59Z

packages/block-library/src/button/index.js


 	styles: [
-		{ name: 'default', label: _x( 'Rounded', 'block style' ), isDefault: true },
+		{ name: 'default', label: _x( 'Default', 'block style' ), isDefault: true },


I think we landed a PR recently where we could drop these entirely from here and they will be added automatically (needs verification)

Can we get a reference to that PR here? What exactly can we drop?

Sure sorry it wasn't that clear.

Here's the PR #12519

I think the entire line can be dropped (the default style)

I just tested this, and it almost works. The block has the correct styles by default, but the "Default" style option does not appear in the style picker:

Since it's not there, if you select an alternate style, there'd be no way to switch back to the default. 😕

So for now at least, I think we do need these lines.

Mmm weird, I though it was the intent of the linked PR. pinging @swissspidy to check my assumptions :) but feel free to move forward with the PR.

Yeah that was the goal of #12519 🤔 No idea why it doesn't work, I would have to investigate.

ZebulanStanphill · 2019-02-08T20:31:47Z

Since the "Rounded" style for buttons is now just "Default", wouldn't it make sense to also add a dedicated "Rounded" style that themes can choose to support if their default isn't rounded? Same for other blocks where the default style wasn't previously just called "Regular".

youknowriad · 2019-02-08T20:55:47Z

@ZebulanStanphill nothing stops themes from registering custom style variations.

ZebulanStanphill · 2019-02-08T21:10:51Z

@youknowriad I know, but I thought it was kind of weird to provide a core style for a long line separator but not a dedicated core style for a short line separator.

mapk

Just tested and everything appears to be working great! I've a few notes.

The is-style-default class didn't appear when I added the block. It only appeared when I selected it from the block style variations. Maybe this is how it always worked?
The Table block show default in the Block Styles, but uses the class is-style-regular in the HTML. Is this right?

kjellr · 2019-02-11T18:22:53Z

The is-style-default class didn't appear when I added the block. It only appeared when I selected it from the block style variations. Maybe this is how it always worked?

Yeah, that's how it behaves today.

The Table block show default in the Block Styles, but uses the class is-style-regular in the HTML. Is this right?

Hmm... you're right. That's not ideal. Ideally we should be using is-style-default there. But changing that up will be a breaking change for some themes, so I'm not sure the best way to handle that. My inclination is to leave it as is, but I could use another viewpoint on that.

mapk · 2019-02-11T20:02:57Z

Thanks for addressing those points, @kjellr.
I don't think number 2 should hold this PR up. LGTM

mapk

Alrighty,

kjellr · 2019-02-14T17:10:00Z

Thanks, everyone!

webmandesign · 2019-02-26T13:51:29Z

Hi guys,

I'm really too late to the party here, sorry, but still I can see an issue with Separator block styles (and probably other blocks too):

styles: [
	{ name: 'default', label: __( 'Default' ), isDefault: true },
	{ name: 'wide', label: __( 'Wide Line' ) },
	{ name: 'dots', label: __( 'Dots' ) },
],

If a theme styles <hr> wide by default (which I'm sure many themes do), than a user is offered 2 same options here: "Defalut" and "Wide line".

Wouldn't this cause UX issues?
How could a theme user select a "Short line" style then?
Or "Short line" style is not available any more now?

swissspidy · 2019-02-26T14:10:54Z

@webmandesign See #14102.

webmandesign · 2019-02-26T15:04:41Z

Thanks @swissspidy

Resolves #13660. This aligns the classname with the actual name we're using in the UI, and also allows for a bit more flexibility for default block styles per theme.

Rename default block styles to "Default"

5a0090d

Resolves #13660. This aligns the classname with the actual name we're using in the UI, and also allows for a bit more flexibility for default block styles per theme.

kjellr added [Feature] Blocks Overall functionality of blocks Needs Design Feedback Needs general design feedback. labels Feb 5, 2019

kjellr self-assigned this Feb 5, 2019

kjellr requested review from jasmussen and mapk February 5, 2019 13:49

kjellr requested review from Soean, ajitbohra, gziolo, jorgefilipecosta and youknowriad as code owners February 5, 2019 13:49

kjellr mentioned this pull request Feb 5, 2019

Default style variation label to match the is-style-default class name #13660

Closed

youknowriad reviewed Feb 5, 2019

View reviewed changes

gziolo added this to the 5.1 (Gutenberg) milestone Feb 5, 2019

mapk removed the Needs Design Feedback Needs general design feedback. label Feb 5, 2019

Merge branch 'master' into update/default-block-style-names

86c43b2

kjellr requested review from noisysocks and talldan as code owners February 6, 2019 18:08

mapk reviewed Feb 11, 2019

View reviewed changes

mapk self-requested a review February 14, 2019 16:45

mapk approved these changes Feb 14, 2019

View reviewed changes

kjellr merged commit baf733c into master Feb 14, 2019

kjellr deleted the update/default-block-style-names branch February 14, 2019 17:10

This was referenced Feb 18, 2019

Bump plugin version to 5.1.0-rc.1 #13908

Merged

Bump plugin version to 5.1.0 #13960

Merged

Conversation

kjellr commented Feb 5, 2019

Uh oh!

jasmussen commented Feb 5, 2019

Uh oh!

youknowriad Feb 5, 2019

Choose a reason for hiding this comment

Uh oh!

mapk Feb 5, 2019

Choose a reason for hiding this comment

Uh oh!

youknowriad Feb 5, 2019

Choose a reason for hiding this comment

Uh oh!

kjellr Feb 5, 2019

Choose a reason for hiding this comment

Uh oh!

youknowriad Feb 5, 2019

Choose a reason for hiding this comment

Uh oh!

swissspidy Feb 9, 2019

Choose a reason for hiding this comment

Uh oh!

ZebulanStanphill commented Feb 8, 2019

Uh oh!

youknowriad commented Feb 8, 2019

Uh oh!

ZebulanStanphill commented Feb 8, 2019

Uh oh!

mapk left a comment

Choose a reason for hiding this comment

Uh oh!

kjellr commented Feb 11, 2019

Uh oh!

mapk commented Feb 11, 2019

Uh oh!

mapk left a comment

Choose a reason for hiding this comment

Uh oh!

kjellr commented Feb 14, 2019

Uh oh!

webmandesign commented Feb 26, 2019

Uh oh!

swissspidy commented Feb 26, 2019

Uh oh!

webmandesign commented Feb 26, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants