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

[iOS]: Fix issue where heading blocks are not behaving properly. #98

Merged
merged 9 commits into from
Dec 17, 2018

Conversation

etoledom
Copy link
Contributor

@etoledom etoledom commented Dec 14, 2018

Fixes wordpress-mobile/gutenberg-mobile#355

This PR forces the typing attributes to be corresponding with the Heading level, when Aztec is used to display Heading blocks.

With @diegoreymendez we looked into the possibility of fixing this on the Aztec level but it easily became super complicated. So we decided for an RNAztecView approach.

The BlockFormatHandler and BlockModel are meant to be easily extensible, in case we need similar implementations with other blocks in the future.

Refertences:

mobile-gutenberg branch: issue/ios-heading-behavior
gutenberg PR: WordPress/gutenberg#12869
Aztec-iOS PR wordpress-mobile/AztecEditor-iOS#1108

heading

To Test:

Setup:

  • Checkout gutenberg-mobile branch issue/ios-heading-behavior.
  • Run git submodule update to checkout submodules commits.
  • Run yarn preios to force update the Aztec version.
  • Run yarn start:reset
  • Open the iOS example project on Xcode open ios/gutenberg.xcodeproj.
  • Clean build folder.
  • Build and run (from Xcode or from yarn ios)

Testing:

  • Create a Heading block.
  • Check that the heading block style is the correct.
  • Type and delete text, change Heading levels, etc...
  • Check that everything looks fine.
  • Format a word of the title with Italic and change Heading levels.
  • Make sure that the Italic format is not removed.
    • Bold has an issue but is already solved on master.

Thank you!

We try to force typing attributes and manually change it on heading level changes.
@etoledom
Copy link
Contributor Author

Sent a couple of commits to solve some issues found by @koke .
I also updated the To test description with an extra setup step and extra details to test.

Copy link
Contributor

@diegoreymendez diegoreymendez left a comment

Choose a reason for hiding this comment

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

I could completely remove the header style by selecting all text and deleting everything. You mentioned in this PR it's a partial fix though so if that's known, everything else looks fine to me.

I also added a comment related to the documentation of a protocol.

@@ -0,0 +1,11 @@
protocol BlockFormatHandler {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it'd be a good idea to document this new protocol to define what its scope is (what does it take care of specifically? what's it purpose?).

This is kinda minor but it's good in making sure all devs stay synchronized moving forward.

@etoledom
Copy link
Contributor Author

Thanks for your review @diegoreymendez

That odd behavior should not be present. At some point it was solved but seems to be there again.
I pushed a commit trying another way to solve the p tags issue, hopefully that helps.

Copy link
Contributor

@diegoreymendez diegoreymendez left a comment

Choose a reason for hiding this comment

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

Works as advertised.

@etoledom etoledom merged commit 90d48e7 into master Dec 17, 2018
@etoledom etoledom deleted the issue/ios-heading-behavior branch December 17, 2018 21:44
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.

[iOS] Heading block glitch
2 participants