Skip to content

Preserve link, code span instances when styling #945

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

Merged
merged 2 commits into from
Nov 25, 2021

Conversation

hypest
Copy link
Contributor

@hypest hypest commented Nov 24, 2021

Fix

PR to preserve the span and tag order for link and code tags. Although HTML doesn't dictate that the inline element order is of importance, there's no good reason for Aztec to alter the order when parsing.

In this PR, the code that applies style to the a and code spans is refactored to be able to set the style on the span without replace the span itself in the spannable.

This PR is an alternative approach of #944.

Test

  1. In MainActivity.kt use the following html as value to the EXAMPLE val: <p>This is a <a href="http://wordpress.com"><strong>link</strong></a></p>
  2. Compile and run the demo app
  3. Switch to html mode and notice that the markup is the same as the one in EXAMPLE
  4. Switch back to visual mode
  5. Type a new character in the beginning of the line
  6. Switch to html mode again
  7. Notice that the strong is still nested inside the a

The above steps fail without the changes in this PR.

Make sure strings will be translated:

  • If there are new strings that have to be translated, I have added them to the client's strings.xml as a part of the integration PR.

@hypest hypest force-pushed the fix/preserve-link-span-order branch 2 times, most recently from 7fd9182 to 5b629b2 Compare November 24, 2021 14:34
@hypest hypest force-pushed the fix/preserve-link-span-order branch from 5b629b2 to 8fe0eb0 Compare November 24, 2021 14:38
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 🎊 ! Thanks @hypest for addressing this issue and adding unit tests 🙇 .

I verified the changes in the Aztec demo app and WPAndroid app as well as, check that the new unit tests passed 🟢 .

Tested on Samsung Galaxy S20 FE 5G (Android 11).

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.

2 participants