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

Dismiss bottom sheet on return key submit #20848

Merged

Conversation

guarani
Copy link
Contributor

@guarani guarani commented Mar 12, 2020

Fixes wordpress-mobile/gutenberg-mobile#1164

Description

Bottom sheets may contain text inputs. When a text input has focus and the keyboard is displayed, tapping the Return button on the keyboard should dismiss the bottom sheet.

How has this been tested?

There are two types of bottom sheets:

  • The link sheet
  • The Button block settings sheet
    (The sheet that the Latest Posts block uses does not include text inputs which use the return key on their keyboard, so its not affected by this change.)

The link sheet appears in any block that includes a rich text area such as Paragraph, Heading, Button, Image (caption), Video (caption), Cover, List, Gallery, Media & Text, etc.
The Button block settings sheet is present only on the Button block.

Paragraph Block

  1. Create a Paragraph Block and add text
  2. Tap the link button in the toolbar to reveal a sheet containing link options
  3. Fill in the first text input
  4. Tap the keyboard Return button and observe that the sheet is dismissed
  5. Repeat steps 3 and 4 for any other text inputs on the sheet

Button Block

  1. Create a Button Block and add text
  2. Tap the link button in the toolbar to reveal a sheet containing link options
  3. Fill in the first text input
  4. Tap the keyboard Return button and observe that the sheet is dismissed
  5. Repeat steps 3 and 4 for any other text inputs on the sheet
  6. Repeat steps 2-5 using the settings button instead of the link button

How to identify link and settings buttons

Screenshots

Paragraph Block Button Block

Types of changes

  • Bug fix
  • New feature

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.

@guarani guarani changed the title Rnmobile/dismiss bottom sheet on submit Dismiss bottom sheet on return key submit Mar 12, 2020
@guarani guarani marked this pull request as ready for review March 13, 2020 00:30
@guarani
Copy link
Contributor Author

guarani commented Mar 13, 2020

I've been unable to test on Android today (only iOS) – if someone can help with that, that would be appreciated.

guarani added 2 commits March 13, 2020 12:13
When the user creates or edits a link in text and a bottom sheet is shown, both the bottom sheet and the keyboard must be dismissed when the keyboard return key is tapped.
There are two scenarios in regards to the Button Block because the link button and the settings button open different bottom sheets. The `isLinkSheetVisible` handles the first scenario and `closeGeneralSidebar` handles the second.
@guarani guarani force-pushed the rnmobile/dismiss-bottom-sheet-on-submit branch from 15c84cb to bfad633 Compare March 13, 2020 15:15
@mchowning
Copy link
Contributor

This is looking good @guarani ! 🎉

I did notice on iOS that when tapping the return key the keyboard would dismiss along with the bottom sheet and then immediately reappear. I don't think this is necessarily a blocker for this PR, but if it's possible to avoid that disappear/reappear it would be nice (could either have the keybaord just disappear and not reappear or could have the keyboard remain open the whole time). FWIW, on Android the keyboard just disappears and does not reappear. What do you think?

@guarani
Copy link
Contributor Author

guarani commented Mar 16, 2020

FWIW, on Android the keyboard just disappears and does not reappear.

Thanks for testing this @mchowning, I'm having trouble getting the Android emulator to run (and I will get myself an Android phone soon).

I did notice on iOS that when tapping the return key the keyboard would dismiss along with the bottom sheet and then immediately reappear.

Yeah, I noticed that too. The existing behavior on iOS (prior to this PR) is:

  1. Keyboard is open, user selects text, taps link toolbar button, bottom sheet is displayed above keyboard.
  2. User makes edits, hits the keyboard return button and only the keyboard is dismissed (this is what we want to fix).
  3. Since the bottom sheet is still open, dragging it down will close it but will also make the keyboard reappear.

I think it makes sense for the keyboard to not reappear, which will bring this in-line with Android. I can look to add the change to this PR since I think it's relevant to the issue.

@guarani
Copy link
Contributor Author

guarani commented Mar 19, 2020

I think it makes sense for the keyboard to not reappear, which will bring this in-line with Android. I can look to add the change to this PR since I think it's relevant to the issue.

In offline discussion, @iamthomasbishop suggested that we in fact keep iOS the way it is, and change Android so that its keyboard reappears when the the bottom sheet is dismissed. The reasoning is that since the block's input was focused before opening the bottom sheet, it should again be focused after closing the sheet.

@guarani
Copy link
Contributor Author

guarani commented Mar 26, 2020

I'd like to ask you for another review please @mchowning as the previous comment has now been resolved as per wordpress-mobile/gutenberg-mobile#1164 (comment).

Could I also ask you to please test on Android, @mchowning?

Copy link
Contributor

@mchowning mchowning 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 to me and we can address the (minor) Android issue separately.

@mchowning mchowning merged commit 712dbfd into WordPress:master Mar 27, 2020
@github-actions github-actions bot added this to the Gutenberg 7.9 milestone Mar 27, 2020
@guarani guarani deleted the rnmobile/dismiss-bottom-sheet-on-submit branch March 27, 2020 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bottom Sheets] Tapping "return" should dismiss sheet
2 participants