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] Add support for "multiple": false block types #28339

Merged
merged 4 commits into from
Jan 23, 2021

Conversation

AmandaRiu
Copy link
Contributor

@AmandaRiu AmandaRiu commented Jan 20, 2021

Related PR

gutenberg-mobile: wordpress-mobile/gutenberg-mobile#3042

Description

Fixes wordpress-mobile/gutenberg-mobile#1156 by adding logic for handing blocks that should only be used once in a single post. These "single use" blocks (such as the "More" block) have the following properties defined in their block.json file:

{
    "supports": {
	"multiple": false
    }
}

The changes in this PR enforce this configuration by disabling the block option if that block already exists in the post.

How has this been tested?

  1. Open the demo app on either android or ios. The demo already has a More block added to the post.
  2. Tap + to open the block menu - notice the More block is disabled. Verify tapping the More option does not work.
  3. Cancel the block menu.
  4. Delete the More block in the post.
  5. Tap + to open the block menu - notice the More block is now enabled. Tap to add this block to the post. Verify success.

Screenshots

finished

iOS: option enabled iOS: option disabled Android: option enabled Android: option disabled
more-enabled-ios more-disabled-ios more-enabled-android more-disabled-android

Types of changes

  • Added new .disabled style for the inserter-button component
  • Passed the item.isDisabled value to the UI (this value already existed)

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.

If the `block.json` configuration for a block specifies
`"multiple":false`, then only a single instance of that block
should be allowed in a post. If the block already exists in the
post, the option to add that block again will be disabled.
@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Jan 20, 2021
@AmandaRiu AmandaRiu changed the title [RNMobile]Add support for "multiple": false block types [RNMobile] Add support for "multiple": false block types Jan 20, 2021
@AmandaRiu
Copy link
Contributor Author

pinging @iamthomasbishop to approve of design for disabled option.

@iamthomasbishop
Copy link

@AmandaRiu This looks nice from a visual design perspective, but I have one suggestion regarding that interaction. I think we may want to show a feedback component like a Snackbar or Compact Notice when the user tries to tap the disabled button, so there is some sort of feedback/indication as to what happened. If we were to use a Compact Notice, the messaging could be something along the lines of "Only one Read More block is allowed on a [post/page]."). Would something like that be doable?

I think a Compact Notice might be easiest to implement, because that component is all-RN, but I'm open to other ideas. Fwiw, I'm also open to shipping the styling improvement now and iterate on the interaction feedback, so whatever works better.

@AmandaRiu AmandaRiu marked this pull request as ready for review January 22, 2021 18:23
@cameronvoell cameronvoell self-requested a review January 22, 2021 18:25
@AmandaRiu
Copy link
Contributor Author

@AmandaRiu This looks nice from a visual design perspective, but I have one suggestion regarding that interaction. I think we may want to show a feedback component like a Snackbar or Compact Notice when the user tries to tap the disabled button, so there is some sort of feedback/indication as to what happened. If we were to use a Compact Notice, the messaging could be something along the lines of "Only one Read More block is allowed on a [post/page]."). Would something like that be doable?

I think a Compact Notice might be easiest to implement, because that component is all-RN, but I'm open to other ideas. Fwiw, I'm also open to shipping the styling improvement now and iterate on the interaction feedback, so whatever works better.

Hi @iamthomasbishop! That seems like it should be doable and since there isn't a huge rush on this task I can take a look at it before it gets merged and see what can be done. Thank you for the review!

Copy link
Member

@cameronvoell cameronvoell left a comment

Choose a reason for hiding this comment

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

Change works great. Nice work @AmandaRiu

Fwiw, I'm also open to shipping the styling improvement now and iterate on the interaction feedback, so whatever works better.

I agree with Thomas that we can ship this improvement and perhaps track interaction feedback as a future improvement (maybe we can create an issue for that).

This fix looks ready to ship :shipit:

@AmandaRiu
Copy link
Contributor Author

Change works great. Nice work @AmandaRiu

Fwiw, I'm also open to shipping the styling improvement now and iterate on the interaction feedback, so whatever works better.

I agree with Thomas that we can ship this improvement and perhaps track interaction feedback as a future improvement (maybe we can create an issue for that).

This fix looks ready to ship :shipit:

Opened this ticket to track these design changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Read More Block: Don't allow multiple
3 participants