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

Implements the editor title in RN for Gutenberg #9027

Merged
merged 13 commits into from
Jan 25, 2019

Conversation

daniloercoli
Copy link
Contributor

@daniloercoli daniloercoli commented Jan 21, 2019

Description:

Implements the editor title in RN for Gutenberg, also removes the native title.

Related PRs:

gutenberg: WordPress/gutenberg#13199
gutenberg-mobile: wordpress-mobile/gutenberg-mobile#450

Testing:

Test 1:

  1. Run the app.
  2. Make sure Gutenberg is enabled.
  3. Open a Gutenberg post and edit the title.
  4. Update the post, close the editor.

Make sure the post title has been udpated.

Test 2:

  1. Run the app.
  2. Make sure Gutenberg is enabled.
  3. Open a Gutenberg post and edit the title.
  4. Close the editor without saving.

Make sure the post title hasn't changed.

Test 3:

  1. Run the app.
  2. Make sure Gutenberg is enabled.
  3. Create a new post.

Make sure the placeholder shows up and the title is empty.

Test 4:

Make sure pressing ENTER does not insert a newline. We will add the correct behavior on a separate PR.

…ss-Android into gutenberg/post-title-in-rn

* 'develop' of https://github.com/wordpress-mobile/WordPress-Android: (41 commits)
  CircleCI: Use explicit Gradle options to prevent out of memory errors
  Avoid using content resolver to get valid mime type, instead use UrlUtils.getUrlMimeType()
  Replace ellipsis with colon
  Update Aztec reference to v1.3.16
  Add newline to tests
  Update the GB mobile ref to point to merged commit
  Keep tab selection in Stats
  Update the GB mobile ref
  Update the GB mobile ref
  Update the GB mobile ref
  Update the GB mobile ref
  Update the GB mobile ref
  Point to GB mobile with JitPack workaround
  Update release notes
  Removed duplicated dots from image URL
  alpha-149 / 671 version bump
  11.6-rc-4 / 670 version bump
  Don't use React Native's version of okhttp3
  Align non-english text in stats to right
  Remove layoutDirection
  ...
@daniloercoli daniloercoli requested a review from mzorz January 24, 2019 14:57
@mzorz mzorz self-assigned this Jan 24, 2019
return mWPAndroidGlueCode.getTitle(new OnGetContentTimeout() {
@Override public void onGetContentTimeout(InterruptedException ie) {
AppLog.e(T.EDITOR, ie);
Thread.currentThread().interrupt();
Copy link
Contributor

Choose a reason for hiding this comment

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

While I can see this has been copied from getContent(), I have to say this caught my attention and had to do some reading about it. TIL.

Copy link
Contributor

@mzorz mzorz left a comment

Choose a reason for hiding this comment

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

Nice work here @daniloercoli and @diegoreymendez !

Tested on the given scenarios and verified all works well on a couple emulators and a handset.

Small thing, when the title is focused, you can still tap on the undo/redo arrows and these work on the block immediately below, like it's focused.
Steps to reproduce:

  1. start a new draft
  2. write a title
  3. also enter some text in the body
  4. tap on the title to focus on it and observe the cursor appears there
  5. tap on undo, observe the block below changes its contents to whatever it had before
  6. tap on redo, observe it again operates on the block below the title.

I've tested Gutenberg on the web and what you do on the title affects the undo/redo history as well, so I think we may want to reflect that behavior here, but I think it's perfectly fine to leave it for a different PR / set of PRs.

Looks good to me! :shipit:

@diegoreymendez
Copy link

@mzorz, @daniloercoli - I've added an item to make sure we implement undo / redo for the title in this issue:

wordpress-mobile/gutenberg-mobile#372

We can spawn more specific issues off that one if needed.

I'd say if possible, let's synch to merge this @daniloercoli, so we can move on the remaining tasks.

…ss-Android into gutenberg/post-title-in-rn

* 'develop' of https://github.com/wordpress-mobile/WordPress-Android: (521 commits)
  Add a unit test to verify non-mobile segments are filtered out
  Remove an unintended release note addition which was most likely caused by a merge
  Update ref to point to merged commit in master
  Remote post conflict resolution uses OnPostChanged
  Post reference for conflict resolution dialog nullified early
  Simplify the undo logic for keeping local version of conflicted post
  Point gutenberg-mobile to release/v0.3.4
  bump aztec version to v1.3.18 (#9103)
  Add NEW_SITE_CREATION_ENABLED build flag
  Revert add site creation to release-notes.xml
  Revert some changes from "Clean up NewSiteCreationBaseFormFragment"
  Revert "Switch to the new site creation flow"
  Revert "Remove classes and resources related to old site creation flow"
  Revert "Remove jumpToNewPost method which isn't used in new site creation flow"
  Revert "Remove unused resources related to old site creation flow"
  updated to latest GB mobile hash
  Fix bug in NewSiteCreationService - service started multiple times
  moved initialization of helper classes to onCreate()
  Update GB Hash
  Add action that commits the changes in the metadata strings files
  ...

# Conflicts:
#	libs/editor/WordPressEditor/build.gradle
@daniloercoli daniloercoli merged commit f3cc48d into develop Jan 25, 2019
@daniloercoli daniloercoli deleted the gutenberg/post-title-in-rn branch January 25, 2019 10:03
@daniloercoli daniloercoli restored the gutenberg/post-title-in-rn branch January 25, 2019 15:23
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.

3 participants