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

Update the columns block to use the new colors hook #21038

Merged
merged 4 commits into from
Mar 25, 2020

Conversation

youknowriad
Copy link
Contributor

@youknowriad youknowriad commented Mar 20, 2020

Based on #21021 to try test the API on different blocks (columns for this PR)

@github-actions
Copy link

github-actions bot commented Mar 20, 2020

Size Change: -42 B (0%)

Total Size: 859 kB

Filename Size Change
build/block-editor/index.js 102 kB -1 B
build/block-library/index.js 110 kB -51 B (0%)
build/block-library/style-rtl.css 7.44 kB +5 B (0%)
build/block-library/style.css 7.45 kB +5 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 998 B 0 B
build/annotations/index.js 3.43 kB 0 B
build/api-fetch/index.js 3.39 kB 0 B
build/autop/index.js 2.58 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.02 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 B 0 B
build/block-editor/style-rtl.css 10.9 kB 0 B
build/block-editor/style.css 10.9 kB 0 B
build/block-library/editor-rtl.css 7.22 kB 0 B
build/block-library/editor.css 7.23 kB 0 B
build/block-library/theme-rtl.css 669 B 0 B
build/block-library/theme.css 671 B 0 B
build/block-serialization-default-parser/index.js 1.65 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 57.5 kB 0 B
build/components/index.js 191 kB 0 B
build/components/style-rtl.css 15.8 kB 0 B
build/components/style.css 15.7 kB 0 B
build/compose/index.js 6.21 kB 0 B
build/core-data/index.js 10.6 kB 0 B
build/data-controls/index.js 1.04 kB 0 B
build/data/index.js 8.25 kB 0 B
build/date/index.js 5.37 kB 0 B
build/deprecated/index.js 771 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.06 kB 0 B
build/edit-post/index.js 91.2 kB 0 B
build/edit-post/style-rtl.css 8.47 kB 0 B
build/edit-post/style.css 8.46 kB 0 B
build/edit-site/index.js 6.72 kB 0 B
build/edit-site/style-rtl.css 2.88 kB 0 B
build/edit-site/style.css 2.88 kB 0 B
build/edit-widgets/index.js 4.43 kB 0 B
build/edit-widgets/style-rtl.css 2.58 kB 0 B
build/edit-widgets/style.css 2.58 kB 0 B
build/editor/editor-styles-rtl.css 428 B 0 B
build/editor/editor-styles.css 431 B 0 B
build/editor/index.js 43.8 kB 0 B
build/editor/style-rtl.css 4 kB 0 B
build/editor/style.css 3.98 kB 0 B
build/element/index.js 4.44 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 6.95 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 1.93 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.49 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.3 kB 0 B
build/keycodes/index.js 1.69 kB 0 B
build/list-reusable-blocks/index.js 2.99 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 4.84 kB 0 B
build/notices/index.js 1.57 kB 0 B
build/nux/index.js 3.01 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.54 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 781 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/rich-text/index.js 14.5 kB 0 B
build/server-side-render/index.js 2.55 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.01 kB 0 B
build/viewport/index.js 1.61 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.18 kB 0 B

compressed-size-action

@youknowriad youknowriad changed the title Update the colors block to use the new colors hook Update the columns block to use the new colors hook Mar 20, 2020
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

Nice, I like the direction.

Unrelated note. The number of experimental props used for RichText and InnerBlocks makes it nearly impossible for 3rd party devs to follow code implementation uses in Gutenberg 🙁 It also means it’s all undocumented and difficult to use for contributors...

packages/block-library/src/columns/style.scss Outdated Show resolved Hide resolved
@youknowriad
Copy link
Contributor Author

The number of experimental props used for RichText and InnerBlocks makes it nearly impossible for 3rd party devs to follow code implementation uses in Gutenberg

True, we need to continue our audits. the color hook is something I want to stabilize pretty quickly

@youknowriad youknowriad force-pushed the update/group-colors branch 2 times, most recently from 92ef4bb to dfca58c Compare March 24, 2020 12:40
@youknowriad youknowriad changed the base branch from update/group-colors to master March 24, 2020 13:37
@youknowriad youknowriad self-assigned this Mar 24, 2020
@youknowriad youknowriad added [Block] Columns Affects the Columns Block [Type] Code Quality Issues or PRs that relate to code quality labels Mar 24, 2020
@youknowriad youknowriad marked this pull request as ready for review March 24, 2020 13:38
@youknowriad
Copy link
Contributor Author

This one is ready to ship.

@youknowriad youknowriad added Needs Dev Ready for, and needs developer efforts Needs Dev Note Requires a developer note for a major WordPress release cycle and removed Needs Dev Ready for, and needs developer efforts labels Mar 24, 2020
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

Indeed it is, nice work 👍

@youknowriad
Copy link
Contributor Author

Oh, it seems we might need a deprecated version here, I'll add it and ship.

@@ -101,7 +101,9 @@ const BlockComponent = forwardRef(
// should only consider tabbables within editable display, since it
// may be the wrapper itself or a side control which triggered the
// focus event, don't unnecessary transition to an inner tabbable.
if ( wrapper.current.contains( document.activeElement ) ) {
if (
isInsideRootBlock( wrapper.current, document.activeElement )
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For me this is a bug that is present on master too. If you want to select a block, you want to select the root block, not an inner one.

@@ -91,7 +91,7 @@ describe( 'Navigating the block hierarchy', () => {
await pressKeyWithModifier( 'ctrl', '`' );
await pressKeyWithModifier( 'ctrl', '`' );
await pressKeyWithModifier( 'ctrl', '`' );
await pressKeyTimes( 'Tab', 4 );
await pressKeyTimes( 'Tab', 5 );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The colors panel is shown before the columns count now.

@youknowriad youknowriad merged commit 7aab279 into master Mar 25, 2020
@youknowriad youknowriad deleted the update/columns-colors branch March 25, 2020 13:18
@github-actions github-actions bot added this to the Gutenberg 7.9 milestone Mar 25, 2020
@youknowriad youknowriad removed the Needs Dev Note Requires a developer note for a major WordPress release cycle label Apr 8, 2020
@mtias mtias mentioned this pull request May 29, 2020
82 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Columns Affects the Columns Block [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants