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] Gallery - Only run the linkTo saving on the web since it's not mobile related #26227

Conversation

jd-alexander
Copy link
Contributor

@jd-alexander jd-alexander commented Oct 16, 2020

Fixes wordpress-mobile/gutenberg-mobile#2735

Description

#25582 introduced changes to utilize the image_default_link_type from options.php to set the default link for galleries.

The issue is that the logic being utilized in the useEffect hook that triggers the saving of the link doesn't work with mobile

So the current solution is to disable this on mobile and only run it on the web.

How has this been tested?

This has been tested using the instructions below.

  1. Add a Gallery Block to the canvas.
  2. Press the undo button.
  3. Notice that the Gallery block is now removed.

Previously, the block wouldn't be removed.

Screenshots

Before After

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.

@jd-alexander jd-alexander added Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) [Block] Gallery Affects the Gallery Block - used to display groups of images labels Oct 16, 2020
@jd-alexander jd-alexander requested a review from mkevins as a code owner October 16, 2020 22:05
Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

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

I'm having trouble undoing a gallery block on the web (although it's now working on mobile):

gallery block not undone on web

I haven't got around to checking if this is an existing issue though.

@guarani
Copy link
Contributor

guarani commented Oct 21, 2020

I haven't got around to checking if this is an existing issue though.

Checked on https://wordpress.org/gutenberg/ and I can reproduce this issue where I'm unable to undo the action of adding a Gallery block, so it isn't related to this PR.

I don't see this issue reported: https://github.com/WordPress/gutenberg/issues?q=is%3Aissue+undo+label%3A%22%5BBlock%5D+Gallery%22+

So I will go ahead and file a new issue for it. Could you please confirm @jd-alexander if you're able to reproduce this?

@guarani guarani self-requested a review October 21, 2020 17:37
Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

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

Approving since as noted in #26227 (comment), I confirmed that the issue where I'm unable to undo the action of adding a Gallery block is not caused by the changes in this PR.

Update: create #26367 to track this bug

Thanks @jd-alexander!

@jd-alexander
Copy link
Contributor Author

Approving since as noted in #26227 (comment), I confirmed that the issue where I'm unable to undo the action of adding a Gallery block is not caused by the changes in this PR.

🙏

Update: create #26367 to track this bug

Thanks, I just checked and I was able to reproduce on the web as well. Super happy this bug was discovered.

Thanks @jd-alexander!

No prob 🙏

@jd-alexander
Copy link
Contributor Author

@guarani I am going to close this PR as this one #26377 resolves the issue for both web and mobile 😄 I just switched to that branch and all was well.

@jd-alexander jd-alexander deleted the rnmobile/fix_gallery_link_to_issue branch October 22, 2020 20:18
@guarani
Copy link
Contributor

guarani commented Oct 23, 2020

Good work catching that! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Gallery Affects the Gallery Block - used to display groups of images 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.

Gallery block is not removed when undo is pressed
2 participants