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 dismiss keyboard button for the post title #17149

Conversation

geriux
Copy link
Member

@geriux geriux commented Aug 22, 2019

Related gutenberg-mobile PR

Description

This fixes an iOS and Android issue with the post title when it is in focus, the option to hide the keyboard wasn't working as expected (it should dismiss the keyboard)

Right now it was only clearing the selected blocks and not the post title so I changed it to dispatch both. If the number of different elements increases another approach should be implemented so it's easy to maintain.

How has this been tested?

  • Open the demo app
  • Tap on the Post title
  • Tap on the "Hide keyboard option"
  • It dismisses the keyboard

Screenshots

Types of changes

Bug fix

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.

jorgefilipecosta and others added 5 commits August 22, 2019 11:51
* Editor: Update the store to use Core Data entities.

* Editor: Fix selector test suites.

* Editor: Fix some legacy selectors and behaviors.

* Editor: Fix action tests.

* Editor: Fix remaining broken unit tests.

* Editor: Fix more tests.

* Editor: Fix more e2e test behaviors.

* Editor: Fix preview functionality.

* Core Data: Fix autosaves filtering.

* Editor: Don't make entity dirty with initial edits.

* Editor: Don't save if the post is not saveable.

* Core Data: Fix merged edits logic.

* Core Data: Fix undo to fit e2e expected behaviors.

* Core Data: Handle more change detection and saving flows.

* Block Editor: Fix undo level logic.

* Core Data: Clean up undo reducer comment.

* Editor: Make `serializeBlocks` a util.

* Core Data: Clarify raw attribute usage.

* Core Data: Memoize .

* Core Data: Use new raw entity record selector instead of modifying the existing one.

* Core Data: Make save notices the caller's responsibility.

* Editor: Use the store key constant in actions instead of a string literal.

* Editor: Defer serialization of blocks until save.

* Editor: Fix raw content access in set up.

* Editor: Revert broken test change.

* Editor: Make initial edits a dirtying operation.

* Editor: Add comment clarifying why we set content to a new function on edits.

* Demo: Fix tests to consider the initial edits dirtying.

* Core Data: Set auto-drafts to drafts when autosaving them.

* Core Data: Handle receiving autosaves correctly when editing non-autosave-persisting-properties.
…nderers (WordPress#16512)

* Add callbacks to ServerSideRenderer to handle failures

* Update variable naming and set default renderers

* LoadingResponsePlaceholder

* swtich case

* fetchComplete

* Remove fetchcomplete method

* Pass render props

* add changelog entry

* add documentation for new props
Previously, tapping at the end of the post would insert a block
immediately after the currently selected block. In addition, this commit
is cleaning out a few unusued props in the block-list file.
@pinarol
Copy link
Contributor

pinarol commented Aug 23, 2019

hi @geriux 👋 Thanks for the PR. This is working good 🎉
But there are some lint issues needs fixing.
You can use npm run lint to detect those, and npm run lint-js:fix to make use of the automation fixing, but it might not fix everything. Travis CI should get green before we can merge.

@pinarol pinarol 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 Aug 23, 2019
@geriux
Copy link
Member Author

geriux commented Aug 23, 2019

Hey @pinarol ! Sorry about that! Linting issues solved =)

@pinarol
Copy link
Contributor

pinarol commented Aug 23, 2019

Thanks!

@pinarol
Copy link
Contributor

pinarol commented Aug 29, 2019

We created a new working branch named rnmobile/master to avoid this issue until it is solved. Could you please update this PR's target branch as rnmobile/master so that our CI tests can pass and we can merge?

@geriux geriux changed the base branch from master to rnmobile/master August 29, 2019 08:32
@geriux
Copy link
Member Author

geriux commented Aug 29, 2019

Of course! I have a conflict after changing the base that I'll fix today =)

@geriux
Copy link
Member Author

geriux commented Aug 29, 2019

@pinarol I see there are a lot of changes when I changed the base, did you mean maybe to make the changes from rnmobile/master instead of what I used before? sorry for the confusion 😅

@pinarol
Copy link
Contributor

pinarol commented Aug 29, 2019

Basically we should base the changes of this PR on 'rnmobile/master' so that we are leaving breaking changes outside. Feel free to do this the easiest way you prefer, you can create a new branch from 'rnmobile/master' and cherry pick this changes onto it.

@geriux
Copy link
Member Author

geriux commented Aug 29, 2019

Oh ok! I went ahead and did that, here's the new one. #17260

@geriux geriux closed this Aug 29, 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.

6 participants