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.12k stars 989 forks source link

PE-424 resize sticker after selecting #425

Closed Yumin2019 closed 2 years ago

Yumin2019 commented 2 years ago
  1. resize sticker after selecting
  2. 5 stickers per a line
Yumin2019 commented 2 years ago

Lint Error on Kotlin environment.

Error: When using `setHasFixedSize() in an RecyclerView, wrap_content cannot be used as a value for size in the scrolling direction. [InvalidSetHasFixedSize from recyclerview-1.2.1]

So I changed the wrap_content attribution to match_parent. And optimization option for sticker, emoji part. (but I need to search about it more)

Yumin2019 commented 2 years ago

if we use many of stickers, we need to load these stickers via sampling. I added optimization processing on it. but we need to use same size pictures.

See below!!

And... Fixed Text part layout error(it had the problem since I started this library) ColorPicker RecyclerView on Top => on Bottom

burhanrashid52 commented 2 years ago

@Yumin2019 Thanks for the PR. It would great if we can fetch the sticker from a remote URL and then scale the loaded bitmap when we add to PhotoEditorView

Yumin2019 commented 2 years ago

From now on, we can add stickers from remote url.

lucianocheng commented 2 years ago

Heads up @burhanrashid52 this change won't make it into the kotlin conversion unless we backmerge it.

burhanrashid52 commented 2 years ago

Heads up @burhanrashid52 this change won't make it into the kotlin conversion unless we backmerge it.

Better to re-merge it

lucianocheng commented 2 years ago

Better to re-merge it

Do you mean, merge it again after we merge in the kotlin branch? If so we should decide who is responsible for tracking and doing that.

Or we just say the Kotlin release won't include it until this PR owner re-merges it.

It may be a non-trivial change, and there may be other changes that pile up before we're done. We're hitting a lot of existing QA issues that aren't passing in the kotlin release.

burhanrashid52 commented 2 years ago

Rebase the existing branch from master and convert it into kotlin. Or do we want to wait for a fully kotlin conversation and then start merging PR ?

lucianocheng commented 2 years ago

Rebase the existing branch from master and convert it into kotlin.

All the touched files in the existing branch will be 100% different, due to the kotlin language switch in the file. This will make the diffs a lot of work. One PR is fine, but if they pile up this will be a lot of work.

Or do we want to wait for a fully kotlin conversation and then start merging PR ?

This PR is an example of the drift I spoke about on Pull Request #407 link to comment. If we continue to have PR's merged in prior to the kotlin conversion, it will create a lot of double work for everyone involved. Potentially so much double work, that the stream of ongoing java-to-kotlin rebease/conversions would grow faster than our ability to merge them in to the kotlin branch.

I still prefer not having a kotlin-release branch and instead, merging in the test and app kotlin changes. That would get around this problem. For this specific PR, the kotlin app changes would have already been merged in.

However if you want to keep a separate branch with the kotlin changes, there's no way to avoid having double work. Either the Kotlin release goes out without the new changes, and individual PR owners need to volunteer to rewrite their changes in Kotlin, or the PRs should wait until the kotlin conversion is done.

burhanrashid52 commented 2 years ago

@lucianocheng Hmm... makes sense...I am fine with without branch approach. However, I have a question that if we go without the branch approach. i.e directly merging to master...How do we handle PR from other members which are in java ?

lucianocheng commented 2 years ago

However, I have a question that if we go without the branch approach. i.e directly merging to master...How do we handle PR from other members which are in java ?

If you're talking about PR's branched before the kotlin conversion merge, then it's no different than PR's now. They would have to pull from the master branch and resolve the conflicts.

If you're talking about new code, I won't stand in it's way but I don't think it will be popular. I'm no longer contributing any Java code so I won't participate in any future Java work.

Fundamentally I'm just asking that the kotlin work follow the same PR process that we currently have, which merges changes into master as they are approved. Having special rules, and a long-running kotlin-conversion branch, creates problems that this repo / project doesn't have a process to deal with.

burhanrashid52 commented 2 years ago

Cool. Then let's keep merging the Kotlin changes in master and will release it when it's done.

burhanrashid52 commented 2 years ago

@lucianocheng I need to resolve the conflicts now.

lucianocheng commented 2 years ago

@lucianocheng I need to resolve the conflicts now.

👍 . We will continue working on getting the core PhotoEditor library code converted. We'll wait on the conflict resolutions to continue beyond that.