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

Make em the default tag for italic #777

Merged
merged 11 commits into from
Feb 18, 2019
Merged

Make em the default tag for italic #777

merged 11 commits into from
Feb 18, 2019

Conversation

Tug
Copy link
Contributor

@Tug Tug commented Jan 22, 2019

Fix

Gutenberg has <em> defined as the tag for italic formatting. Aztec can handle <em> but converts it to <i> when sending back the html to gutenberg-mobile. This causes inconsistencies in the UI and should be addressed.

Test

  1. Checkout gutenberg-mobile PR Use format-library for the formatting bar gutenberg-mobile#275
  2. Use the following patch to make changes to Aztec on gutenberg-mobile:
curl "https://gist.githubusercontent.com/Tug/50f0a51b7833a94d5f05f4fa7020a289/raw/3317f9ab4dd096cb2a358c44bb5e1085a8a15550/debug-with-aztec.patch" | git apply -v
  1. Run the app and check that italic formatting is selected when the cursor/selection is on italic
    formatting

Review

@hypest

@Tug Tug self-assigned this Jan 22, 2019
@hypest
Copy link
Contributor

hypest commented Feb 6, 2019

Updated the PR, expanded it to make <em> be the default for italic and recused the tests to test for such. Ready for a review.

👋 @daniloercoli , you think you can make a review pass on this one? Thanks!

@daniloercoli
Copy link
Contributor

Nice! With tests updated this PR can be 🚢ed!

Feel free to merge once the conflicts are resolved.

@hypest
Copy link
Contributor

hypest commented Feb 7, 2019

Thanks for the review @daniloercoli !

Let's hold off merging this until the parent PR is also ready to merge wordpress-mobile/gutenberg-mobile#275. This way we can avoid a potential revert in case the parent PR doesn't get merged.

@hypest
Copy link
Contributor

hypest commented Feb 18, 2019

We're giving the "go ahead with merging" for the Gutenberg and gutenberg-mobile side PRs so, merging this.

@hypest hypest merged commit e02ec13 into develop Feb 18, 2019
@hypest hypest deleted the update/italic-tag-em branch February 18, 2019 13:36
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.

3 participants