Pullquote: add block support #30951

ramonjd · 2021-04-19T05:21:20Z

Description

In this PR, we're adding border and color block support to the Pullquote block.

Given that we can now achieve border and background variations using block supports, we're removing the existing styles and adding a textAlign attribute to compensate.

Furthermore, because the new block supports already offers the controls we need to style the Pullquote, the current controls are not required. We have therefore removed those and also the styles/border/classes logic.

ℹ️ There was some discussion about whether this PR could be split into two: adding first border support, then color support in a later PR. I believe this block is a special case because, at present, the background colour control determines both the border colour and background colour, depending on which block style we select.

How has this been tested?

Manually, in the editor (web and mobile device)

Because this PR deprecates the current version of Pullquote, we've updated the fixtures (version 4 is the last previous version).

Check that fixtures are valid:

npm run fixtures:regenerate test/integration/full-content/full-content.test.js

And by pasting current fixture block code into the editor and making sure a valid transform takes place.

Check in a native mobile device that the block works as intended.

Screenshots

Types of changes

These changes add experimental border and color block support to the Pullquote block.

The attributes are changing, hence we've added a deprecation (v4).

Block support controls supersede the block's original controls, so we have removed them. So too, there's now no longer any need to differentiate between the usage of mainColor for background and border, since we can control each independently.

Part of #28913

cc @aaronrobertshaw

packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-4.parsed.json

packages/e2e-tests/fixtures/blocks/core__pullquote__main-color.json

packages/e2e-tests/fixtures/blocks/core__pullquote__main-color.parsed.json

ockham · 2021-04-19T12:36:56Z

This might not be specific to this block, but if I set a border color, width, and radius, and leave the border style as 'Default', I see something like this:

That looks a bit off. If I change the border style to 'Solid', it looks right:

packages/block-library/src/pullquote/deprecated.js

packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-3.serialized.html

packages/e2e-tests/fixtures/blocks/core__pullquote__main-color__deprecated-4.serialized.html

ramonjd · 2021-04-20T05:14:48Z

This might not be specific to this block, but if I set a border color, width, and radius, and leave the border style as 'Default', I see something like this:

It's the style variant CSS driving this I think. It's my opinion that we can remove the style variations in the next iteration as mention in the description.

packages/block-library/src/pullquote/block.json

packages/e2e-tests/fixtures/blocks/core__pullquote__deprecated-4.parsed.json

aaronrobertshaw · 2021-04-20T07:10:07Z

This might not be specific to this block, but if I set a border color, width, and radius, and leave the border style as 'Default', I see something like this:

It's the style variant CSS driving this I think. It's my opinion that we can remove the style variations in the next iteration as mention in the description.

I believe this is a theme issue. The theme sets a solid border style for the top and bottom borders. The "Solid color" block style has more specific CSS that sets border: none.

packages/block-library/src/pullquote/block.json

packages/block-library/src/pullquote/deprecated.js

ramonjd · 2021-07-12T05:00:39Z

@ramonjd Can you replicate the duplicate panel issue?

Thanks for the ping. I've just rebased the branch and have started retesting.

quote-example-after-rebase.mp4

I haven't come across the double color panel conundrum yet.

aaronrobertshaw

I've given this another test after the latest rebase. Everything works as advertised for me.

Still no sign of a duplicate color panel. I'm comfortable saying that was a one-off in Joen's local environment.

There are also a couple of e2e failures still. Re-running the tests didn't get them passing. The failures do appear unrelated to this PR so once they pass we should be right to merge.

Thanks for all the hard work on this @ramonjd 👍

ramonjd · 2021-07-19T22:45:55Z

There are also a couple of e2e failures still. Re-running the tests didn't get them passing. The failures do appear unrelated to this PR so once they pass we should be right to merge.

Thanks a lot for the final check @aaronrobertshaw !

Looks like some of the fixtures are now failing 🤷 Maybe that's the cause of the E2E failures. I'll rebase and 🤞

ramonjd · 2021-07-20T00:40:16Z

Merged changes from #33502 which move block e2e fixtures

Adding some TODOs for later cleanups

…ustom colors in the deprecation and newer versions. Removing hanging empty blocks in parsed JSON

save: to make blockquote class assignment more consistent with figure class assignment migrate: to make customMainColor/style logic neater and clearer Remove state wrapper around pullquote export Rebuilding fixtures with the (for now) expected hanging block in parsed.json files. We could trim them in `getBlockFixtureHTML`

…or deprecation v3 where there is no solid style, a mainColor attribute and an existing inline border style on the figure. Regnerated fixtures. Removing textColor from block attributes as they're adding by supports hooks

…unction require it, are returning the required attributes for the latest version.

… the default and solid block styles were, aside from the text alignment, redundant. So we have removed them. To compensate for the alignment, this commit adds a text alignment block control. Deprecated blocks will still have the `is-style-solid-class`, so we keep the styles targeting it, and also ensure that we set the new attribute `textAlign` to "left" when it's present. Add specificity to the left and right text align styles so that the block alignment controls have a great chance of overriding theme styles.

All we needed to do was add `__deprecated` to the file name so that the fixture test regex could find it :)

ramonjd requested review from ajitbohra, nerrad, oandregal, ntwb and spacedmonkey as code owners April 19, 2021 05:21

ramonjd force-pushed the add/pullquote-block-border-color-support branch 3 times, most recently from 4b873c9 to a1acc07 Compare April 19, 2021 05:39

ockham mentioned this pull request Apr 19, 2021

Next iteration of block supports mechanism #28913

Closed

20 tasks