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

[Gutenberg] [Aztec] Prevent headings from becoming bold after words are autocorrected #17844

Merged
merged 6 commits into from
Feb 16, 2022

Conversation

twstokes
Copy link
Contributor

@twstokes twstokes commented Jan 31, 2022

This PR upgrades Aztec to version 1.19.8 and points to the Gutenberg Mobile commit that incorporates this change.

Related:

To test:

Prerequisites:

  1. Create a new Post
  2. Add a Heading block
  3. In the block type: "Hello i'm testing a heading." (autocorrect should intervene on "i'm")
  4. Observe the heading is now "Hello I'm testing a heading."
  5. Observe that when highlighting "I'm", the bold formatting button isn't depressed / enabled.
  6. Publish the Post.
  7. Observe that "I'm" isn't shown in bold on the site. (you can further inspect the HTML source to ensure <strong> tags don't wrap the text)

Regression Notes

  1. Potential unintended areas of impact
  • Any text block in the block editor
  • Replacing text in text blocks
  • Autocorrecting any text block
  1. What I did to test those areas of impact (or what existing automated tests I relied on)
  • Used testing steps above
  • Tested various non-heading blocks
  1. What automated tests I added (or what prevented me from doing so)

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@twstokes twstokes changed the title Point to Aztec bold heading fix commit. Prevent headings from becoming bold after words are autocorrected Jan 31, 2022
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Jan 31, 2022

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@twstokes twstokes requested a review from guarani January 31, 2022 20:03
@jkmassel
Copy link
Contributor

jkmassel commented Jan 31, 2022

You can test the WordPress changes on this Pull Request by downloading it from AppCenter here with build number: pr17844-2d3eeda. IPA is available here. If you need access to this, you can ask a maintainer to add you.

@jkmassel
Copy link
Contributor

jkmassel commented Jan 31, 2022

You can test the Jetpack changes on this Pull Request by downloading it from AppCenter here with build number: pr17844-2d3eeda. IPA is available here. If you need access to this, you can ask a maintainer to add you.

Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

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

Just leaving a review here. I tested this yesterday in the WPiOS app and it worked well, but when testing in Aztec I noticed a potential blocker: wordpress-mobile/AztecEditor-iOS#1334 (review).
Feel free to re-request a review when ready.

@twstokes
Copy link
Contributor Author

twstokes commented Feb 4, 2022

I've pointed to the latest Aztec commit that hopefully mitigates the issue found.

@twstokes twstokes requested a review from guarani February 4, 2022 16:26
Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

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

Approved via wordpress-mobile/AztecEditor-iOS#1334 (review).
I also tested here with the installable build and the testing steps work as expected and I didn't notice any regressions.

@twstokes
Copy link
Contributor Author

twstokes commented Feb 11, 2022

Awaiting approvals on the Gutenberg Mobile side before cutting the new Aztec release. Once the release is cut we'll point to the tag.

@twstokes
Copy link
Contributor Author

twstokes commented Feb 14, 2022

This branch was caught up with the 19.2 release branch so that it would use GB Mobile v1.71.1. The release has not yet been merged back into trunk.

@twstokes twstokes changed the title Prevent headings from becoming bold after words are autocorrected [Gutenberg] [Aztec] Prevent headings from becoming bold after words are autocorrected Feb 14, 2022
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Feb 14, 2022

You can test the WordPress changes on this Pull Request by downloading it from AppCenter here with build number: pr17844-8943e0c. IPA is available here. If you need access to this, you can ask a maintainer to add you.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Feb 14, 2022

You can test the Jetpack changes on this Pull Request by downloading it from AppCenter here with build number: pr17844-8943e0c. IPA is available here. If you need access to this, you can ask a maintainer to add you.

@twstokes
Copy link
Contributor Author

twstokes commented Feb 15, 2022

@fluiddot, since you helped out with the related Gutenberg + Gutenberg Mobile PRs, could you check this PR as well? I have the Podfile pointed to the merged Gutenberg Mobile commit, which if I understand correctly, may need to be tagged before this PR is merged into trunk. Thank you! 🙇

@twstokes twstokes requested a review from fluiddot February 15, 2022 14:46
@twstokes
Copy link
Contributor Author

twstokes commented Feb 15, 2022

⚠️ I just did a quick test of the generated binary on this PR and it failed - the bold heading issue still appeared. I'm looking into it. 👀

@@ -4,6 +4,7 @@
* [*] Stats: fix navigation between Stats tab. [#17894]
* [*] Add "Copy Link" functionality to Posts List and Pages List [#17911]
* [*] [Jetpack-only] Enables the ability to use and create WordPress.com sites, and enables the Reader tab. [#17914, #17948]
* [*] Block editor: Autocorrected Headings no longer apply bold formatting if they weren't already bold. [#17844]
Copy link
Contributor

Choose a reason for hiding this comment

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

If the Gutenberg Mobile ref is not being updated, which would include this fix, I'm wondering whether we should add this entry, wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point @fluiddot - I'd leave it out if we were just updating WordPress-Editor-iOS. After our discussion, though, the plan will be to include the GB Mobile upgrade as well with this PR.

@twstokes
Copy link
Contributor Author

twstokes commented Feb 15, 2022

⚠️ I just did a quick test of the generated binary on this PR and it failed - the bold heading issue still appeared. I'm looking into it. 👀

This is working now that the GB Mobile bundle has been generated and we're pointing to that commit. 👍 After the Gutenberg Mobile PR is merged and a tag is created, we'll update this PR.

Copy link
Contributor

@fluiddot fluiddot left a comment

Choose a reason for hiding this comment

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

LGTM 🎊 !

I confirm that the issue can't be reproduced in the build pr17844-d47c82c.

NOTE: Totally unrelated to this PR but I noticed that the picker for switching the site doesn't work in the installable build. Not sure if it's a known issue otherwise we should open it.

@fluiddot
Copy link
Contributor

As a side note, it would be great to address the warning from Peril (reference) and assign a milestone before merging the PR.

Screenshot 2022-02-16 at 14 18 43

@twstokes twstokes added this to the 19.3 milestone Feb 16, 2022
@twstokes
Copy link
Contributor Author

NOTE: Totally unrelated to this PR but I noticed that the picker for switching the site doesn't work in the installable build. Not sure if it's a known issue otherwise we should open it.

Thanks for bringing this up @fluiddot! I'm running the same build and haven't been able to reproduce that. 😬

@twstokes
Copy link
Contributor Author

This has been pointed to the GB Mobile tag.

@twstokes twstokes self-assigned this Feb 16, 2022
@twstokes twstokes merged commit d4e8240 into trunk Feb 16, 2022
@twstokes twstokes deleted the bug/aztec-headings branch February 16, 2022 16:16
SiobhyB pushed a commit that referenced this pull request Mar 3, 2022
[Gutenberg] [Aztec] Prevent headings from becoming bold after words are autocorrected
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants