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

[Mobile] Insert new block below post title if post title is selected #16440

Merged
merged 2 commits into from Jul 9, 2019

Conversation

@mchowning
Copy link
Contributor

commented Jul 5, 2019

For mobile, this changes the behavior when the user selects a post's title and inserts a block. Previously, the new block would be inserted at the bottom of the post (this will remain the behavior on web). This PR changes this so that on mobile the new block will be inserted at the top of the post immediately before the title.

Before After
before mp4 after mp4

How has this been tested?

See related mobile-gutenberg PR

Types of changes

The starting point for determining the insertion location for a new block is the index of the currently selected block. If either (a) the post's Title was currently selected, or (b) nothing in the post was selected, the value for the index of the selected block will be -1. The insertion index was then incremented and, if that resulted in an indexAfterSelected value of 0, the block would be inserted at the end of the post instead (indexAfterSelected || this.props.blockCount). As a result, if either the post's Title was selected or no block was selected, a new block would be inserted at the end of the post. Otherwise, the block would be inserted immediately after the currently selected block.

This PR seeks to

  1. Change the behavior when the post's Title is selected (insert the new block at the beginning of the post);
  2. Maintain the current behavior in the case where no blocks are selected (insert the new block at the end of the post); and
  3. Maintain the current behavior in the case where a block is selected (insert the new black after the selected block).

It does this by lifting the state of whether the post title is selected out of the post-title component to the visual-editor component so that it can be shared with the block-list component, which handles inserting newly added blocks.

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.

@mchowning mchowning requested review from etoledom and marecar3 Jul 5, 2019

@mchowning mchowning self-assigned this Jul 5, 2019

Insert new block below post title if post title is selected
Previously, the new block would have been inserted at the end of the
post, which is almost always going to be off-screen on mobile.

@mchowning mchowning force-pushed the rnmobile/insert-post-from-title-at-start branch from ad58aa2 to 933150f Jul 7, 2019

@etoledom
Copy link
Contributor

left a comment

Working great as described. Nice job @mchowning 🎉
Left a code comment, but other than that I'd say is good to merge

I saw a weird behavior but then I noticed that it's related to this issue wordpress-mobile/gutenberg-mobile#932 , so we can handle it on a separated PR.

The issue is that after inserting an image block (or any that doesn't auto-focus), the title remains selected.
After that, when adding another block, the ADD BLOCK HERE marker shows a different position than where the block is finally inserted.

add-block

Fix blockCount reference
Co-Authored-By: etoledom <etoledom@icloud.com>

@mchowning mchowning force-pushed the rnmobile/insert-post-from-title-at-start branch from 00dbad2 to 925e92f Jul 8, 2019

@mchowning mchowning changed the title Insert new block below post title if post title is selected [Mobile] Insert new block below post title if post title is selected Jul 8, 2019

@mchowning mchowning requested a review from etoledom Jul 8, 2019

@etoledom
Copy link
Contributor

left a comment

Thank you for the update. Looking great! 🎉

Let's wait for the gutenberg-mobile v1.9 code-freeze to finish to merge this one.
wordpress-mobile/gutenberg-mobile#1201 (review)

@mchowning mchowning merged commit ceacc10 into master Jul 9, 2019

1 of 4 checks passed

Filter merged Filter merged
Details
Filter merged Filter merged
Details
Filter merged Filter merged
Details
Travis CI - Pull Request Build Passed
Details

@github-actions github-actions bot added this to the Gutenberg 6.1 milestone Jul 9, 2019

@mchowning mchowning deleted the rnmobile/insert-post-from-title-at-start branch Jul 9, 2019

jg314 added a commit to jg314/gutenberg that referenced this pull request Jul 19, 2019

[Mobile] Insert new block below post title if post title is selected (W…
…ordPress#16440)

* Insert new block below post title if post title is selected

Previously, the new block would have been inserted at the end of the
post, which is almost always going to be off-screen on mobile.

* Fix blockCount reference

Co-Authored-By: etoledom <etoledom@icloud.com>

sbardian added a commit to sbardian/gutenberg that referenced this pull request Jul 29, 2019

[Mobile] Insert new block below post title if post title is selected (W…
…ordPress#16440)

* Insert new block below post title if post title is selected

Previously, the new block would have been inserted at the end of the
post, which is almost always going to be off-screen on mobile.

* Fix blockCount reference

Co-Authored-By: etoledom <etoledom@icloud.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.