Implement keyboardDidHide listener and open modal if the Keyboard appears #10
Conversation
|
@danieldoglas when user currently focuses on input and select picker and close picker, should we focus back to original input? |
|
@0xmiroslav I don't think we should go back to the original input, we should just close. I'm addressing internally why I can't assign you to this PR, I think we have some permission issues here. Will get back to you soon. |
|
@0xmiroslav can you try to review this PR without me assigning you? |
Sure thing |
0xmiros
left a comment
0xmiros
left a comment
There was a problem hiding this comment.
Code mostly looks good. Just minor feedback:
src/index.js
Outdated
|
|
||
| if (Keyboard.isVisible()) { | ||
| const keyboardListener = Keyboard.addListener('keyboardDidHide', () => { | ||
| this.updatePickerState(animate,postToggleCallback); |
There was a problem hiding this comment.
| this.updatePickerState(animate,postToggleCallback); | |
| this.updatePickerState(animate, postToggleCallback); |
src/index.js
Outdated
| Keyboard.dismiss(); | ||
| } | ||
| else { | ||
| this.updatePickerState(animate,postToggleCallback); |
There was a problem hiding this comment.
| this.updatePickerState(animate,postToggleCallback); | |
| this.updatePickerState(animate, postToggleCallback); |
src/index.js
Outdated
| } | ||
| else { |
|
all comments have already been resolved |
|
@OlimpiaZurek please pull from master and fix lint. Not related to this PR but asking since you also worked on #9. - const preserveSpaces = (label) => {
+ const preserveSpaces = (label) => {
return label.replace(/ /g, '\u00a0');
- }
+ }And it would be good to add some automated tests. |
done |
0xmiros
left a comment
0xmiros
left a comment
There was a problem hiding this comment.
Looks good, tests well, automated tests pass.
Checklist will be done in app PR.
cc: @danieldoglas
3 participants
Original issue: $Expensify/App#16353
This PR changes implementation on Keyboard dismiss inside
togglePickermethod. In current implementationKeyboard.dismiss()is called immediately, which can caused issues ( eg. Expensify/App#16353).I added listener
keyboardDidHideand open modal if the Keyboard appears. Otherwise just open modal.