search

wordpress-mobile / AztecEditor-Android

A reusable native Android rich text editor component.
Mozilla Public License 2.0
673 stars 114 forks source link

Added backgroundColorSpan support v3 #1041

Closed yuvalgnessin-qz closed 1 year ago

yuvalgnessin-qz commented 1 year ago

This PR is meant to replace #934 and #911

It contains the same changes, but it's been updated from trunk.

Feature

Test

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

Review

@SiobhyB

device-2020-05-26-184604

yuvalgnessin-qz commented 1 year ago

@SiobhyB thanks for your comments; you were right about many of the merge artifacts and the test change should never have happened; I think it was working around a bug.

I'm looking into the Plugin issue requested by Cameron this was addressed by ab286ede56b5534212bde3917d112d8c29f97cad

So I believe everything should be addressed, but it seems that your CI isn't running on my branch. Please let me know if there's anything else!

yuvalgnessin-qz commented 1 year ago

Thank you @SiobhyB and @planarvoid. Please do try to get to this as soon as you can so that we can finally put it behind us!

yuvalgnessin-qz commented 1 year ago

@SiobhyB @planarvoid Any update on this?

Sorry to be a bother, but I was hopeful that the 3rd time will be the charm and that we can finally get this merged upstream after my most recent changes. Unfortunately my branch is once again out of date with trunk. I'd be happy to update it one final time if you can dedicate the effort required on your end to get it merged, but otherwise I cannot promise to continue maintaining this PR.

Thank you!

SiobhyB commented 1 year ago

@yuvalgnessin-qz, this PR's still on our radar, though I understand if you're not available to keep checks on it. The PR itself isn't currently showing any conflicts with trunk, but if conflicts do come up and you're not available then I'd be happy to update it prior to merge.

yuvalgnessin-qz commented 1 year ago

@SiobhyB thanks for the response! I'm going to take this off my queue for now, but please ping me when you want to take next steps and hopefully I can jump back in. Thanks!

SiobhyB commented 1 year ago

As an update: @planarvoid is currently on leave, so won't be able to look at this any time in the coming months. However, in our brief discussion, he did note that he thinks this PR should be ready to be merged given all the factors outlined here. With all the previous reviews and the fact this works in my own testing too, I'm going to go ahead to merge this.

@yuvalgnessin-qz, thank you again for all your time and work on this!

yuvalgnessin-qz commented 1 year ago

That's great news! Thanks @SiobhyB 🎉

If you don't mind, please let me know when you release the next version of AztecEditor-Android - when that happens, I'd like to migrate our project off of our custom fork.

Thank you!

SiobhyB commented 9 months ago

@yuvalgnessin-qz, 👋 , just in case you missed it, the v1.7.0 release includes your changes. :)