search

wordpress-mobile / AztecEditor-Android

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

Add possibility to use a custom layout for the toolbar #916

Closed starshipcoder closed 2 years ago

starshipcoder commented 3 years ago

Permits to ui client to change icon order, hide icon, use 2 lines...

maxme commented 3 years ago

Just a note that we're changing the project license, since this haven't landed yet it's not a problem, but if the patch land after the switch, it will be done under the MPL v2.

Ref. https://github.com/wordpress-mobile/AztecEditor-Android/pull/922

starshipcoder commented 3 years ago

No problem for me, it is even better!

maxme commented 3 years ago

@starshipcoder that works fine, but the logic is a bit weird to me. What about keeping the advanced option and add a customLayout attribute that, if set, would override the advanced option.

I think @anitaa1990 worked on that, what's your opinion about this one?

starshipcoder commented 3 years ago

You are right ! I will do that. Thank you for the review

anitaa1990 commented 3 years ago

@starshipcoder that works fine, but the logic is a bit weird to me. What about keeping the advanced option and add a customLayout attribute that, if set, would override the advanced option. I think @anitaa1990 worked on that, what's your opinion about this one?

That sounds like a good idea to me!

dimanych commented 3 years ago

Please 👍 and merge this pr )

starshipcoder commented 3 years ago

I apply @maxme suggestion, would you mind review the PR ?

oguzkocer commented 2 years ago

@anitaa1990 It looks like this PR might have slipped through the cracks, could you take a look at it when you get a chance?

anitaa1990 commented 2 years ago

I can take a look at this one today 👍

anitaa1990 commented 2 years ago

@oguzkocer there are a few checks failing in the PR (related to Buildkite). Is there something I need to do (maybe create the same branch customize_toolbar_layout in order to reinitiate the checks again?

oguzkocer commented 2 years ago

@anitaa1990 The CI checks don't work in forked PRs, a separate PR needs to be created to run the checks. I'll send you a Slack message with a link to a guide for this.