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] Fix link "open in new tab" switch #15812

Merged
merged 3 commits into from
May 27, 2019

Conversation

etoledom
Copy link
Contributor

@etoledom etoledom commented May 24, 2019

Fixes wordpress-mobile/gutenberg-mobile#1018

gutenberg-mobile side PR: wordpress-mobile/gutenberg-mobile#1030

This PR fixes the "Open in New Tab" option for links in gutenberg-mobile, where the switch started always off, even a link target was set.

link-switch

To test:

  • Open the example app.
  • Select text on a paragraph block and add a link attribute to it.
  • Switch the "Open in New Tab" option ON.
  • Close the Bottom Sheet.
  • Open the Link options again for the same text.
  • Check that the Switch is ON.

@etoledom etoledom added the Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) label May 24, 2019
@etoledom etoledom self-assigned this May 24, 2019
@etoledom etoledom requested a review from mkevins May 24, 2019 13:13
@etoledom etoledom marked this pull request as ready for review May 24, 2019 13:17
@@ -53,10 +53,13 @@ class ModalLinkUI extends Component {
return;
}

const { activeAttributes: { url, target } } = this.props;
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this refactor.

Copy link
Contributor

@mkevins mkevins left a comment

Choose a reason for hiding this comment

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

I tested this on the gutenberg-mobile demo app for both Android and iOS, and this fixes the issue as described.

Good work 🎉 LGTM!

@hypest
Copy link
Contributor

hypest commented May 27, 2019

I'll move this PR forward since Eduardo is AFK.

@hypest hypest merged commit c4cd4d8 into master May 27, 2019
@hypest hypest deleted the rnmobile/1018-link-open-new-tab branch May 27, 2019 09:42
@youknowriad youknowriad added this to the Gutenberg 5.9 milestone Jun 7, 2019
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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Open in New Tab" setting gets reset when link settings are re-opened
4 participants