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

[RNMobile] Create link settings component #23299

Merged
merged 5 commits into from
Jun 24, 2020
Merged

Conversation

lukewalczak
Copy link
Member

@lukewalczak lukewalczak commented Jun 19, 2020

That PR introduces a new component called LinkSettings to handle link configuration in blocks such as Buttons and Social Link and probably in a few more in the future.

ref to gb mobile: wordpress-mobile/gutenberg-mobile#2406

Description

Refactored Button block to extract code related to link configuration and collect it in new component called LinkSettings.

The most important prop in new component is options which is an object of options, eg:

		const options = {
			url: {
				showIcon: true,
				label: __( 'Link URL' ),
				placeholder: __( 'Add URL' ),
				autoFocus: false,
				autoFill: true,
			},
			openInNewTab: {
				showIcon: true,
				label: __( 'Open in new tab' ),
			},
			linkRel: {
				showIcon: true,
				label: __( 'Link Rel' ),
				placeholder: __( 'None' ),
			},
		};

How has this been tested?

Go through test steps Button-6 and Button-7 from sanity-tests

Screenshots

Types of changes

Refactor - Button block
Feature - new component LinkSettings

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.

@lukewalczak lukewalczak added [Type] Enhancement A suggestion for improvement. Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) labels Jun 19, 2020
@lukewalczak lukewalczak self-assigned this Jun 19, 2020
@github-actions
Copy link

github-actions bot commented Jun 19, 2020

Size Change: 0 B

Total Size: 1.13 MB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 3.4 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.37 kB 0 B
build/block-directory/style-rtl.css 941 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 108 kB 0 B
build/block-editor/style-rtl.css 10.7 kB 0 B
build/block-editor/style.css 10.7 kB 0 B
build/block-library/editor-rtl.css 7.59 kB 0 B
build/block-library/editor.css 7.6 kB 0 B
build/block-library/index.js 129 kB 0 B
build/block-library/style-rtl.css 8.04 kB 0 B
build/block-library/style.css 8.04 kB 0 B
build/block-library/theme-rtl.css 730 B 0 B
build/block-library/theme.css 732 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 197 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.62 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.44 kB 0 B
build/date/index.js 5.47 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.19 kB 0 B
build/edit-navigation/index.js 9.87 kB 0 B
build/edit-navigation/style-rtl.css 1.02 kB 0 B
build/edit-navigation/style.css 1.02 kB 0 B
build/edit-post/index.js 303 kB 0 B
build/edit-post/style-rtl.css 5.5 kB 0 B
build/edit-post/style.css 5.5 kB 0 B
build/edit-site/index.js 16.6 kB 0 B
build/edit-site/style-rtl.css 3.02 kB 0 B
build/edit-site/style.css 3.02 kB 0 B
build/edit-widgets/index.js 9.32 kB 0 B
build/edit-widgets/style-rtl.css 2.42 kB 0 B
build/edit-widgets/style.css 2.42 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 44.9 kB 0 B
build/editor/style-rtl.css 3.81 kB 0 B
build/editor/style.css 3.81 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.73 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 710 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.13 kB 0 B
build/list-reusable-blocks/style-rtl.css 450 B 0 B
build/list-reusable-blocks/style.css 451 B 0 B
build/media-utils/index.js 5.29 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 663 B 0 B
build/nux/style.css 660 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.5 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 14 kB 0 B
build/server-side-render/index.js 2.68 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.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

Copy link
Member

@geriux geriux left a comment

Choose a reason for hiding this comment

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

Hey Luke 👋

The code looks good! Nice to see some of the code moved into a new component to be reusable.

Regarding the sanity tests, I found one issue:

Button-6

  • Buttons block - Settings: Open in new tab - steps
  • ⚠️ Buttons block - Settings: Link rel - steps -> This one fails, I can't update the value.
  • Buttons block - Settings: Link URL - steps
  • Buttons block - Settings: Remove link - steps
  • Buttons block - Settings: Synchronize with button options - steps

Button-7

  • Buttons block - Link from the clipboard is automatically added into the empty URL field in link settings and button options - steps
  • Buttons block - Toolbar link button is active when Button has link - steps

@lukewalczak
Copy link
Member Author

@geriux Thanks for CR! Will investigate it asap!

@lukewalczak
Copy link
Member Author

@geriux Wrong value was passed to rel TextControl. I don't know why it wasn't pushed before 🤔

Would you mind running a check on that one more time?

@geriux
Copy link
Member

geriux commented Jun 24, 2020

@geriux Wrong value was passed to rel TextControl. I don't know why it wasn't pushed before 🤔

Would you mind running a check on that one more time?

Sure! Can you please update the Gutenberg Mobile PR with develop? Since you merge with master I'd need the monorepo changes there. Thanks!

@geriux
Copy link
Member

geriux commented Jun 24, 2020

Thanks for the changes!

  • Buttons block - Settings: Link rel - steps

LGTM! Tested it on the demo app and on WordPress iOS.

@lukewalczak lukewalczak merged commit c70aba7 into master Jun 24, 2020
@lukewalczak lukewalczak deleted the rnmobile/link-settings branch June 24, 2020 12:59
@github-actions github-actions bot added this to the Gutenberg 8.5 milestone Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants