burhanrashid52 / PhotoEditor

A Photo Editor library with simple, easy support for image editing using paints,text,filters,emoji and Sticker like stories.
MIT License
4.08k stars 983 forks source link

Adjustment made for opacity functionality #495

Closed gabrielnakhata closed 1 year ago

gabrielnakhata commented 1 year ago

Adjustment made for opacity functionality, as already discussed in issue #467, but not merged, thereby discovering and verifying compliance. Previously filed in issue #493

gabrielnakhata commented 1 year ago

@wasky, can you help me to carry out this review, please I am having difficulty dealing with this quality standard.

wasky commented 1 year ago

Third commit is missing after force-push. Also tests in DrawingViewApiTest will fail after introducing changes from third commit. I suggest to use ShapeBuilder.DEFAULT_SHAPE_OPACITY ?: 0 in all places where tests will fail.

wasky commented 1 year ago

In the current version of the pull request there are still two changes missing (they were introduced in the third commit before force-push):

gabrielnakhata commented 1 year ago

I'll check here now, I had to go back to the commit, unfortunately...

wasky commented 1 year ago

Now the code looks good to me, but I'm afraid one test still fails in DrawingViewApiTest.

gabrielnakhata commented 1 year ago

@wasky , I made the corrections for the build, I believe it will reduce the restrictions a lot...

gabrielnakhata commented 1 year ago

@wasky , You're right, I went back to the last step, I have no experience with tests, I read the test build suggestions, but ended up messing with other features... help me close this test, please.

wasky commented 1 year ago

You need to make just one-line change and everything will work fine: In DrawingViewApiTest:112 from ShapeBuilder.DEFAULT_SHAPE_OPACITY ?:0 to ShapeBuilder.DEFAULT_SHAPE_OPACITY.

gabrielnakhata commented 1 year ago

I was suspicious of that line... :)

gabrielnakhata commented 1 year ago

@wasky , Sorry, it looks like two points were deleted, I adjusted and resubmitted.

gabrielnakhata commented 1 year ago

GOOD

gabrielnakhata commented 1 year ago

@burhanrashid52 @wasky , I understand, if I need to change, I am aware of the possible change.

gabrielnakhata commented 1 year ago

Good evening, @burhanrashid52 , The merge was done but it still does not reflect the adjustments for the use of the published library. Strange that also on my fork when I request a "pull origin master" I don't get the settings already merged;

burhanrashid52 commented 1 year ago

To answer your first question, I am waiting for #492 to get merged, and then I will publish it as a new version altogether. To answer second, I've rebased and merged it on master, so you might need to update your fork accordingly.