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

Lighter block DOM: Navigation #20729

Merged
merged 6 commits into from
Mar 10, 2020
Merged

Lighter block DOM: Navigation #20729

merged 6 commits into from
Mar 10, 2020

Conversation

ellatrix
Copy link
Member

@ellatrix ellatrix commented Mar 9, 2020

Description

Makes the navigation block (and inner blocks) light blocks. The goal is to have the same structure as the front-end.

Especially worth noting is the fact that we can now use the ul and li tags. This is something that wasn't possible before because the list items couldn't be direct children of ul.

Screenshot 2020-03-09 at 15 42 18

How has this been tested?

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@github-actions
Copy link

github-actions bot commented Mar 9, 2020

Size Change: -270 B (0%)

Total Size: 864 kB

Filename Size Change
build/block-editor/index.js 105 kB +28 B (0%)
build/block-library/editor-rtl.css 7.26 kB -98 B (1%)
build/block-library/editor.css 7.26 kB -97 B (1%)
build/block-library/index.js 115 kB +77 B (0%)
build/block-library/style-rtl.css 7.38 kB -90 B (1%)
build/block-library/style.css 7.39 kB -90 B (1%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.01 kB 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.6 kB 0 B
build/block-editor/style.css 10.5 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.7 kB 0 B
build/components/index.js 191 kB 0 B
build/components/style-rtl.css 15.5 kB 0 B
build/components/style.css 15.5 kB 0 B
build/compose/index.js 5.75 kB 0 B
build/core-data/index.js 10.6 kB 0 B
build/data-controls/index.js 1.03 kB 0 B
build/data/index.js 8.22 kB 0 B
build/date/index.js 5.36 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.06 kB 0 B
build/edit-post/index.js 91.3 kB 0 B
build/edit-post/style-rtl.css 8.64 kB 0 B
build/edit-post/style.css 8.64 kB 0 B
build/edit-site/index.js 4.64 kB 0 B
build/edit-site/style-rtl.css 2.48 kB 0 B
build/edit-site/style.css 2.48 kB 0 B
build/edit-widgets/index.js 4.44 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 381 B 0 B
build/editor/editor-styles.css 382 B 0 B
build/editor/index.js 43.8 kB 0 B
build/editor/style-rtl.css 3.98 kB 0 B
build/editor/style.css 3.97 kB 0 B
build/element/index.js 4.45 kB 0 B
build/escape-html/index.js 734 B 0 B
build/format-library/index.js 7.09 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.92 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.68 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.85 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.49 kB 0 B
build/priority-queue/index.js 780 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/rich-text/index.js 14.3 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 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

@ellatrix
Copy link
Member Author

This is looking pretty good and I don't see any regressions. It may be that there's a slight difference in the editor, but nothing that I'm seeing right now. If there's a regression, I'll create a follow up PR. As noted above, I believe this is a big step in the direction of being able to share more styles between the front-end and editor.

@ellatrix ellatrix merged commit 03e3c05 into master Mar 10, 2020
@ellatrix ellatrix deleted the try/light-navigation branch March 10, 2020 10:28
@github-actions github-actions bot added this to the Gutenberg 7.8 milestone Mar 10, 2020
@jasmussen
Copy link
Contributor

This caused a small regression.

Before:

Screenshot 2020-03-10 at 12 22 15

After:

Screenshot 2020-03-10 at 12 22 28

It appears this rule is at fault:

Screenshot 2020-03-10 at 12 25 53

The width of the .wp-block-navigation-link must be auto, or it won't work.

@ellatrix
Copy link
Member Author

@jasmussen Strange, for me the inserter icon appeared horizontally. I'll check again.

@ellatrix
Copy link
Member Author

@jasmussen I'm seeing this:

Screenshot 2020-03-10 at 12 32 00

@jasmussen
Copy link
Contributor

Oohh it may be a TwentyNineteen issue. Let me verify.

@ellatrix
Copy link
Member Author

Oooh, sorry, I didn't pull the new commits yet.

@jasmussen
Copy link
Contributor

Confirmed 😱

Thanks for bearing with my bad report here! Though I do suspect that as the DOM gets closer to the frontend (yaaay) we'll see more and more of these things pop up due to themes doing their best to style the old markup.

@ellatrix
Copy link
Member Author

Pulled the commits, and it's still fine:

Screenshot 2020-03-10 at 12 34 45

@jasmussen
Copy link
Contributor

Yep, it's TwentyNineteen. I'll create a trac ticket when I have a moment.

@ellatrix
Copy link
Member Author

@jasmussen Tbh, I think we should attempt to also remove the appender from the editor DOM, and float it behind the last items or something, similar to the sibling inserter (in between inserter). The appender shouldn't be part of the content and take up space. We have a similar problem in the column block where the appender continuously changes the height of the column without any actual content being changed.

@jasmussen
Copy link
Contributor

I've no strong opionion there. But would you suggest not creating a trac ticket to fix this in TwentyNineteen?

@ellatrix
Copy link
Member Author

@jasmussen Oh, I'm not opposed to patching anything up in the meantime. :)

@jasmussen
Copy link
Contributor

Created https://core.trac.wordpress.org/ticket/49613 to track it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants