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] Rich Image Caption feature branch #15571

Merged
merged 8 commits into from
May 16, 2019
Merged

Conversation

etoledom
Copy link
Contributor

@etoledom etoledom commented May 10, 2019

This PR merges the Rich Image Caption feature branch.

All this code has been already tested and approved: #14883

A simple smoke test should be enough to ✅
For more detailed test cases, please check out #14883

gutenberg-mobile side PR: wordpress-mobile/gutenberg-mobile#975

cc @mchowning

mchowning and others added 5 commits April 27, 2019 10:30
* Add ability to style image caption

Adds ability to style image caption with bold, italic, links, and strikethrough (wordpress-mobile/gutenberg-mobile#574).
Adds ability for image captions to wrap to multiple lines instead of getting
cut off (wordpress-mobile/gutenberg-mobile#590).

* Address style errors from linter

* Hide image toolbar controls if caption selected

* Update state in componentDidMount

The previous approach of setting the child component's `isSelected` prop
at the time the child was rendered based on
`this.props.isSelected && this.state.isCaptionSelected` did not work when:

  (a) the child was selected (so its `isSelected` prop was true);
  (b) the child component had no text; and
  (c) the parent's `isSelected` prop was changed to false
      (i.e., another block was selected).

Because the child component is not rendered when both the parent's `isSelected`
prop is false and the caption does not contain any text, the child
component's `onBlur` prop function (the `onCaptionBlur function) would not
be called and update the `isCaptionSelected` state to be false.

This bug shows when a user selects an empty caption, then selects a different
block (thereby hiding the caption since it is empty), and then re-selects the
image with the empty caption. In that scenario, upon re-selecting the image
with the empty caption, the empty caption would appear and immediately be
selected instead of just appearing. This bug would not appear if there
was any text in the caption, because then the caption would always
render and its `onBlur` prop function would be called.

* Add comment explaining isSelected guard

* Update image caption tagname from p to figcaption

This change brings mobile's handling of image captinos in line
with the web. It also addresses a crash that was occurring when
the enter key was tapped to enter a new line in an image caption.

* Remove `onBlurCaption` function

On iOS the display of the link UI modal was causing the `onBlurCaption`
function to be called, which would update the image caption's `isSelected`
prop to false. That would, in turn, immediately remove the
just-displayed modal. Restructuring the logic to not like this to not
use the image caption's `onBlur` function avoids that issue.

* Remove tagName from caption RichText component

Explicitly setting the tagName to figcaption caused an
invalid block to be saved if a caption ended with an empty line.
@etoledom etoledom self-assigned this May 10, 2019
@etoledom etoledom 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 May 10, 2019
@etoledom etoledom requested a review from hypest May 10, 2019 17:31
@etoledom etoledom marked this pull request as ready for review May 10, 2019 17:35
etoledom added 2 commits May 13, 2019 19:48
This style was fetched from the Image block, and it was removed after replacing the TextInput with RichText on captions
@marecar3 marecar3 self-requested a review May 14, 2019 16:31
Copy link
Contributor

@marecar3 marecar3 left a comment

Choose a reason for hiding this comment

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

LGTM!

@etoledom etoledom merged commit f257ed2 into master May 16, 2019
@etoledom
Copy link
Contributor Author

Thank you @marecar3 !

@etoledom etoledom deleted the rnmobile/feature-caption branch May 16, 2019 09:13
@gziolo gziolo added this to the 5.8 (Gutenberg) milestone May 17, 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.

4 participants