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

Add missing heading options #1377

Merged
merged 2 commits into from
Sep 25, 2019

Conversation

SergioEstevao
Copy link
Contributor

@SergioEstevao SergioEstevao commented Sep 23, 2019

Fixes #1261

As discussed with @iamthomasbishop on the ticket above we decided to implement the missing heading options by just adding them to the format toolbar.

Related GB PR: WordPress/gutenberg#17533

Simulator Screen Shot - iPhone 11 - 2019-09-23 at 22 08 31

To test:

  • Start the demo app
  • Check that the heading block has the H4, H5, H6 buttons when selected
  • Use the option and check that the HTML is correctly generated.

Update release notes:

  • If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

@SergioEstevao SergioEstevao added the Aztec Parity Feature exists in Aztec but not yet in Gutenberg mobile label Sep 23, 2019
@SergioEstevao SergioEstevao added this to the Open Beta milestone Sep 23, 2019
Copy link
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

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

Works great on both iOS and Android 🎉

This is how it looks in both platforms:

Headings

✅ for behavior and code.

Let's ask @iamthomasbishop about the looks of it 👍

@iamthomasbishop
Copy link
Contributor

Looking mostly good, I have a couple thoughts.

The font sizes seem a bit small, esp on Android. Here's what I've got them defined as on the design side (in Figma):

  • h1: 24
  • h2: 22
  • h3: 20
  • h4: 18
  • h5: 16 (same as "normal" p text)
  • h6: 14

Also, as noted in this issue, the inside block content areas are a bit off. This throws off the vertical rhythm substantially on Android.

@SergioEstevao
Copy link
Contributor Author

@iamthomasbishop I just confirmed that in iOS the sizes match you specify above.

@SergioEstevao SergioEstevao merged commit dd3a6d6 into develop Sep 25, 2019
@SergioEstevao SergioEstevao deleted the issue/1261_add_missing_header_options branch September 25, 2019 21:53
@iamthomasbishop
Copy link
Contributor

@SergioEstevao can you investigate why Android looks different? They look off in the screenshot above but I’m not sure if that’s my eyes playing tricks?

@hypest
Copy link
Contributor

hypest commented Sep 26, 2019

👋 @SergioEstevao , love to see this merged! 🎉

I wonder, do you think we could add one or more automated tests in this one? Maybe an Appium e2e test?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Aztec Parity Feature exists in Aztec but not yet in Gutenberg mobile
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add missing heading levels
4 participants