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

Fix block movers in navigator on experimental navigation page displaying horizontally #23779

Merged
merged 2 commits into from
Jul 8, 2020

Conversation

talldan
Copy link
Contributor

@talldan talldan commented Jul 8, 2020

Description

Fixes two small issues from #23416.

  1. BlockMobileToolbar (which is the web-based responsive mobile toolbar) had a moverDirection prop, which wasn't being used and didn't do anything, as BlockMover handles its own orientation so this can be omitted.
  2. BlockMoverUpButton and BlockMoverDownButton are both used in the block navigator on the experimental navigation page. The prop for these components should have been renamed to orientation from __experimentalOrientation but was somehow missed. This fixes an issue where the block movers in the navigator are sometimes shown horizontally instead of vertically—the prop is used to force the movers to display vertically.

How has this been tested?

  1. Open up the experimental navigation page and create a menu with a few links
  2. Try the movers in the Navigation Structure area.
  3. Observe the movers are correctly oriented.

Screenshots

Before

Screenshot 2020-07-08 at 12 53 30 pm

After

Screenshot 2020-07-08 at 12 57 32 pm

Types of changes

Bug fix (non-breaking change which fixes an issue)

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.

@talldan talldan added [Type] Bug An existing feature does not function as intended [Feature] Blocks Overall functionality of blocks [Feature] Navigation Screen labels Jul 8, 2020
@talldan talldan self-assigned this Jul 8, 2020
@talldan talldan changed the title Fix block movers in navigator on experimental Fix block movers in navigator on experimental navigation page displaying horizontally Jul 8, 2020
@github-actions
Copy link

github-actions bot commented Jul 8, 2020

Size Change: +4 B (0%)

Total Size: 1.14 MB

Filename Size Change
build/block-editor/index.js 115 kB +4 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.67 kB 0 B
build/api-fetch/index.js 3.39 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 7.67 kB 0 B
build/block-directory/style-rtl.css 944 B 0 B
build/block-directory/style.css 945 B 0 B
build/block-editor/style-rtl.css 10.8 kB 0 B
build/block-editor/style.css 10.8 kB 0 B
build/block-library/editor-rtl.css 7.54 kB 0 B
build/block-library/editor.css 7.54 kB 0 B
build/block-library/index.js 130 kB 0 B
build/block-library/style-rtl.css 7.75 kB 0 B
build/block-library/style.css 7.76 kB 0 B
build/block-library/theme-rtl.css 728 B 0 B
build/block-library/theme.css 729 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.2 kB 0 B
build/components/index.js 198 kB 0 B
build/components/style-rtl.css 15.8 kB 0 B
build/components/style.css 15.8 kB 0 B
build/compose/index.js 9.56 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.46 kB 0 B
build/date/index.js 5.38 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.23 kB 0 B
build/edit-navigation/index.js 10.8 kB 0 B
build/edit-navigation/style-rtl.css 1.08 kB 0 B
build/edit-navigation/style.css 1.08 kB 0 B
build/edit-post/index.js 304 kB 0 B
build/edit-post/style-rtl.css 5.57 kB 0 B
build/edit-post/style.css 5.57 kB 0 B
build/edit-site/index.js 16.6 kB 0 B
build/edit-site/style-rtl.css 3.03 kB 0 B
build/edit-site/style.css 3.03 kB 0 B
build/edit-widgets/index.js 9.35 kB 0 B
build/edit-widgets/style-rtl.css 2.45 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/index.js 45 kB 0 B
build/editor/style-rtl.css 3.78 kB 0 B
build/editor/style.css 3.78 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.71 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 709 B 0 B
build/keyboard-shortcuts/index.js 2.52 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.32 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.4 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 13.9 kB 0 B
build/server-side-render/index.js 2.71 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.13 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@@ -8,18 +8,15 @@ import { useViewportMatch } from '@wordpress/compose';
*/
import BlockMover from '../block-mover';

function BlockMobileToolbar( { clientId, moverDirection } ) {
function BlockMobileToolbar( { clientId } ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this file can actually be deleted. I couldn't see any usage other than the native mobile version of it.

Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't deleting it break the native mobile app, then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's what I don't know. I don't think so as there's a separate index.native.js.

Copy link
Member

Choose a reason for hiding this comment

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

Aha, gotcha. Yeah you're right that it probably can be removed.

Copy link
Member

@noisysocks noisysocks left a comment

Choose a reason for hiding this comment

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

Looks good!

@talldan talldan merged commit b166d5d into master Jul 8, 2020
@talldan talldan deleted the fix/incorrect-orientation-props branch July 8, 2020 05:39
@github-actions github-actions bot added this to the Gutenberg 8.6 milestone Jul 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants