Added backgroundColorSpan support v2

[yuvalgnessin-qz]

This PR is meant to replace #911 since @felipevaladares was no longer available to complete the requested changes.

This branch is forked from his and addresses all remaining comments from @cameronvoell from that PR, including fixing the bug that keeps the background button highlighted.

All changes since #911 are implemented by @lvcasasanta

Feature

• Added CssBackgroundColorPlugin based on CssUnderlinePlugin
• Added AztecBackgroundColorSpan
• General changes to implement and use BackgroundColorSpan

Test

1. Run the app
2. Select some text
3. Click on the new button that is located between Bold and Quote

Review

@cameronvoell

[yuvalgnessin-qz]

@jd-alexander since you seem to be the most active maintainer currently on this project, do you think you might be able to look at this pull request as well?

I'm also hoping you could help me fix the issue with Connected Tests on CI. The output is:

*** WARNING : deprecated key derivation used.
Using -iter or -pbkdf2 would be better.
bad decrypt
140156466037888:error:06065064:digital envelope routines:EVP_DecryptFinal_ex:bad decrypt:../crypto/evp/evp_enc.c:570:

Exited with code exit status 1
CircleCI received exit code 1

[mchowning]

Since you reviewed the previous PR, could you take a look at this one as well @cameronvoell ?

[yuvalgnessin-qz]

Thanks for taking a look @cameronvoell and @mchowning! Any ETA on when this might be reviewed?

[cameronvoell]

@yuvalgnessin-qz Thanks for your patience. I have this in my queue and should be able to get to it before the end of this week, and hopefully earlier. Looking forward to reviewing the updates. 🙇

[yuvalgnessin-qz]

Hi again @cameronvoell - do you still think you might be able to get to this today, or will it have to wait until next week?

No worries either way; I'm sure you're very busy!

[cameronvoell]

Hi again @cameronvoell - do you still think you might be able to get to this today, or will it have to wait until next

Hi @yuvalgnessin-qz See my comment here: https://github.com/wordpress-mobile/AztecEditor-Android/pull/934#discussion_r571833320

My testing looked good for the other concerns so far, there is just that one issue in that comment above that needs to be addressed. I'll test some more this week to see if there are any other concerns.

[yuvalgnessin-qz]

Thanks for the feedback @cameronvoell! I'll try to find time to address your concern today.

FWIW, as of last week we have already shipped an app to production that uses the code in this branch, and so far it seems stable!

[yuvalgnessin-qz]

@cameronvoell I just pushed a commit with the changes you requested. Now, the background color will not appear unless it is added as a plugin.

In order to make this work, I had to modify the logic in AztecToolbar.addButton(). This must be the first time that a toolbar action has been added as a plugin, because it didn't work out of the box. Luckily, the tests I had written in 4261e37 caught these bugs as soon as they were introduced.

Check out the last commit (ab286ed) for the changes in question. Hopefully now we are close to getting this merged!

[yuvalgnessin-qz]

Hi again @cameronvoell! Any updates?

[yuvalgnessin-qz]

Hello again @cameronvoell! Do you think there's any chance of having this merged upstream at some point? Or are you and your team too busy right now? If so, I can close the PR - let me know.

[cameronvoell]

Hi @yuvalgnessin-qz Sorry I was MIA for a bit. The changes you made look good. Unfortunately I'm seeing some failing end to end or "connected" tests. CircleCI is failing on this PR because it's coming from a fork, so that's fine, but you can see on this other branch I made that when connected tests are run here, we are seeing some failures: https://app.circleci.com/pipelines/github/wordpress-mobile/AztecEditor-Android/464/workflows/69fa00ae-551c-4e6d-9b06-1043f15945dc/jobs/1329

You can run those tests locally using./gradlew cAT in the root folder AztecEditor-Android. More details here: https://github.com/wordpress-mobile/AztecEditor-Android#before-running-instrumentation-tests

As far as I can tell the failing tests are all crashing at this line here: https://github.com/wordpress-mobile/AztecEditor-Android/blob/5d983f84e74d0d6db4297e846ef010a3d5146612/aztec/src/main/kotlin/org/wordpress/aztec/toolbar/AztecToolbar.kt#L499-L500 I'll dig around a bit to see why thats occurring in the test, to see if it's catching something that can occur in regular usage, or if it's caused by the conditions of the test.

Apologize for the gap in the reviews, I can check more regularly moving forward on this.

[imthiaz]

@yuvalgnessin-qz Could you please share your source code gradle link so that i can locally execute the color span functionality with my application. i have been using the aztec toolbar in my application.

[yuvalgnessin-qz]

@imthiaz you can view our fork and its releases here and here, and use jitpack to package it through github the same way you would the wordpress artifact.

Something like this:

repositories {
    maven { url 'https://jitpack.io' } // For projects hosted on github
}

dependencies {
    implementation("com.github.quizlet:AztecEditor-Android:background-color-support-v2.1")
}

You are free to use it, but I cannot make any guarantees regarding how long we will be maintaining this fork and whether we will be updating it from the upstream project, nor can I make any guarantees regarding what versioning scheme we may use if we do make changes in the future. For now, it is not in active development. The best long-term approach for maintainability would be for Wordpress to merge this PR upstream so that we can both depend on the original dependency, rather than on the fork.

[imthiaz]

Sure thanks for your quick response and I wil use the mentioned one and test it with my application.

I wish to have the PR merged asap👍👍

On Thu, 8 Apr, 2021, 12:22 am Yuval Gnessin, @.***> wrote:

@imthiaz https://github.com/imthiaz you can view our fork and its releases here https://github.com/quizlet/AztecEditor-Android/tree/feature/background-color-span-support and here https://github.com/quizlet/AztecEditor-Android/releases, and use jitpack to package it through github the same way you would the wordpress artifact.

Something like this:

repositories { maven { url 'https://jitpack.io' } // For projects hosted on github }

...

dependencies { implementation("com.github.quizlet:AztecEditor-Android:background-color-support-v2.1") }

You are free to use it, but I cannot make any guarantees regarding how long we will be maintaining this fork and whether we will be updating it from the upstream project, nor can I make any guarantees regarding what versioning scheme we may use if we do make changes in the future. For now, it is not in active development. The best long-term approach for maintainability would be for Wordpress to merge this PR upstream so that we can both depend on the original dependency, rather than on the fork.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/wordpress-mobile/AztecEditor-Android/pull/934#issuecomment-815146020, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA4HS4OPZIXBLKYBI2EWM4LTHSSWXANCNFSM4WPIIBIA .

[yuvalgnessin-qz]

@imthiaz you can set the background color to whatever you want using AztecText.setBackgroundSpanColor()

[imthiaz]

@imthiaz you can set the background color to whatever you want using AztecText.setBackgroundSpanColor()

Ok thanks for the update.

@yuvalgnessin-qz Do we also support font color change from your version of aztec, as i could see in the above mentioned first AztecDEMO screenshot some text in green color - i.e Italic (Red), Underline (Green with underline.)

Can you let me know how to input such green colored words using aztec if possible.

Initially i asssumed aztec android version does not support font color options

[yuvalgnessin-qz]

@imthiaz I am not sure if Aztec supports text color changing, because that's not something I use it for. It looks like the underline text you asked about in the demo is hardcoded with HTML like this: "<u style=\"color:lime\">Underline</u><br>". You can see that here. I don't know if Aztec supports changing it via toolbar.

This pull request is for background color support only. I am not a maintainer of this library and have no intention of adding any other features to it at this time. If you have any questions that are not related to background color, please open an Issue on the github project. And if there are any missing features that you'd like to add, feel free to open a pull request!

[imthiaz]

Sure understood 👍

On Sat, 10 Apr, 2021, 3:11 am Yuval Gnessin, @.***> wrote:

@imthiaz https://github.com/imthiaz I am not sure if Aztec supports text color changing, because that's not something I use it for. It looks like the underline text you asked about in the demo is hardcoded with HTML like this: "<u style=\"color:lime\">Underline
". You can see that here https://github.com/wordpress-mobile/AztecEditor-Android/pull/934/files#diff-f560a91308e1ade3461de1cb5c543283d28801832ecf0a1475be13ce4eae23ecR93. I don't know if Aztec supports changing it via toolbar.

This pull request is for background color support only. I am not a maintainer of this library and have no intention of adding any other features to it at this time. If you have any questions that are not related to background color, please open an Issue on the github project. And if there are any missing features that you'd like to add, feel free to open a pull request!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/wordpress-mobile/AztecEditor-Android/pull/934#issuecomment-816986877, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA4HS4JNLJLF4KMT5WOZU3DTH5YAHANCNFSM4WPIIBIA .

[yuvalgnessin-qz]

Hello @oguzkocer @fluiddot @planarvoid @khaykov !

I understand that between the time this PR was opened and approximately October 2021, this project was inconsistently maintained. However, I'm happy to see that for the past year or so there have been regular releases.

If I update this branch against trunk, would you all consider merging it upstream, and if necessary, help me get it over the finish line?

Thank you!

[oguzkocer]

Hi there @yuvalgnessin-qz 👋 I am sorry about the late reply. I was on an extended leave at the time of your ping.

I brought this up internally and @SiobhyB graciously volunteered with the review process. I think she is planning to leave a reply as well.

[yuvalgnessin-qz]

@oguzkocer thanks for the reply, and welcome back. I did in fact update my fork from trunk back in January when I wrote the comment, but the changes are in another branch. I would be happy to either update this branch with those changes or open a new PR from the updated branch.

@SiobhyB if I do that, will you be able to help with next steps to get this merged upstream? Thanks!

[SiobhyB]

👋 @yuvalgnessin-qz, thanks for your persistence and patience with these changes!

I'd be happy to work towards getting this merged with you. Merging the latest changes from trunk would be a welcome starting point before I'm able to finish catching up on all the discussions so far. 🙇‍♀️

[yuvalgnessin-qz]

@SiobhyB sounds good. I'll let you know when that's done, then hand it off to you. Thanks!

[yuvalgnessin-qz]

Hi @SiobhyB, I decided to open a fresh PR off of a new branch. Here it is: #1041

[SiobhyB]

@yuvalgnessin-qz, thank you! I'll go ahead to close this one.