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 - Insert a new Paragraph block when pressing Enter on end of captions #22928

Merged
merged 7 commits into from
Jun 8, 2020

Conversation

SergioEstevao
Copy link
Contributor

Description

Updated the Image block on mobile to have the same behavior as the web, and insert a new paragraph block when pressing Enter.

I also added the same behavior for the Video block captions on mobile and web (@ellatrix any reason this was not done when added to the Image block?)

How has this been tested?

This can be tested on GB-mobile using this PR: wordpress-mobile/gutenberg-mobile#2352

Screenshots

Types of changes

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.

@SergioEstevao SergioEstevao 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 Jun 5, 2020
@SergioEstevao SergioEstevao requested review from ellatrix and geriux June 5, 2020 09:24
@github-actions
Copy link

github-actions bot commented Jun 5, 2020

Size Change: +765 B (0%)

Total Size: 1.13 MB

Filename Size Change
build/api-fetch/index.js 3.4 kB +1 B
build/block-directory/index.js 6.77 kB +1 B
build/block-editor/index.js 106 kB +50 B (0%)
build/block-library/index.js 127 kB +208 B (0%)
build/components/index.js 194 kB +17 B (0%)
build/compose/index.js 9.31 kB +2 B (0%)
build/edit-post/index.js 303 kB +26 B (0%)
build/edit-post/style-rtl.css 5.6 kB +168 B (3%)
build/edit-post/style.css 5.6 kB +168 B (3%)
build/edit-site/index.js 15.5 kB -1 B
build/edit-widgets/index.js 9.34 kB +1 B
build/editor/index.js 44.8 kB +120 B (0%)
build/element/index.js 4.65 kB +1 B
build/format-library/index.js 7.72 kB -1 B
build/list-reusable-blocks/index.js 3.12 kB +1 B
build/media-utils/index.js 5.3 kB -1 B
build/plugins/index.js 2.56 kB -1 B
build/primitives/index.js 1.5 kB +2 B (0%)
build/rich-text/index.js 14.8 kB -1 B
build/server-side-render/index.js 2.68 kB +4 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/autop/index.js 2.83 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/style-rtl.css 892 B 0 B
build/block-directory/style.css 892 B 0 B
build/block-editor/style-rtl.css 11.4 kB 0 B
build/block-editor/style.css 11.4 kB 0 B
build/block-library/editor-rtl.css 7.87 kB 0 B
build/block-library/editor.css 7.87 kB 0 B
build/block-library/style-rtl.css 7.72 kB 0 B
build/block-library/style.css 7.72 kB 0 B
build/block-library/theme-rtl.css 684 B 0 B
build/block-library/theme.css 686 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/style-rtl.css 19.5 kB 0 B
build/components/style.css 19.5 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.45 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 771 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.17 kB 0 B
build/edit-navigation/index.js 8.25 kB 0 B
build/edit-navigation/style-rtl.css 918 B 0 B
build/edit-navigation/style.css 919 B 0 B
build/edit-site/style-rtl.css 2.96 kB 0 B
build/edit-site/style.css 2.96 kB 0 B
build/edit-widgets/style-rtl.css 2.4 kB 0 B
build/edit-widgets/style.css 2.4 kB 0 B
build/editor/editor-styles-rtl.css 425 B 0 B
build/editor/editor-styles.css 428 B 0 B
build/editor/style-rtl.css 4.26 kB 0 B
build/editor/style.css 4.27 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 621 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.41 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@ellatrix
Copy link
Member

ellatrix commented Jun 5, 2020

I created #22934 to add it to the other blocks. :)

@SergioEstevao
Copy link
Contributor Author

I created #22934 to add it to the other blocks. :)

I will revert the video block web changes in this PR then.

insertBlocksAfter(
createBlock( 'core/paragraph' )
)
}
Copy link
Contributor

@mchowning mchowning Jun 5, 2020

Choose a reason for hiding this comment

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

I don't think the changes in this file have any effect on mobile because mobile uses a couple the mobile-only caption components (BlockCaption, which wraps Caption)in video/edit.native.js. Just noting that for mobile we have those separate components for captions, whereas iirc on web the captions were being created from scratch (although it sounds like that may be changing).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea was to implement the caption break on video web, but it looks #22934 addressed that. So this will go away!

@mchowning
Copy link
Contributor

This wasn't working for me on Android, but adding the deleteEnter prop in 4c35926 fixed that. I went ahead and pushed that to this branch, but feel free to revert if you disagree with that change for any reason.

I also noticed that the captions for a Gallery block (either for the individual images or for the gallery as a whole) do not seem to be splitting. It seems like we would want the caption for the entire gallery block to split, but it looks like #22934 may address that.

@SergioEstevao
Copy link
Contributor Author

@mchowning it looks your fix for Android is working great!

I also added support for the caption in the gallery. But only for the caption for the full gallery.

Do you mind to give it another look?

@SergioEstevao
Copy link
Contributor Author

@mchowning after merging master to this branch we now also the same feature in pullquote and quote name citations.

@mchowning
Copy link
Contributor

mchowning commented Jun 8, 2020

I'm seeing that the paragraph block only gets created if the cursor is at the end of the caption when you press Enter, if it is anywhere else, then pressing Enter does nothing. I'm seeing this on both platforms with the image, video, and gallery block captions.

caption_middle_split mp4

UPDATE: @SergioEstevao alerted me to the fact that this is the same behavior that we have on the web. Doesn't feel like great UX to me, but even if we wanted to address that, this PR is not the place to address that. Just bringing us in line is a great step forward, so 👍

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.

LGTM! :shipit:

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.

3 participants