Skip to content

Conversation

@panuavakul
Copy link
Contributor

@panuavakul panuavakul commented Dec 17, 2025

PR Checklist (Required)

Please ensure your PR meets the following requirements:

  • The commit message follows our guidelines: https://github.com/hm21/pro_image_editor/blob/stable/CONTRIBUTING.md#style-guides
  • I have run dart format . in the terminal and committed any changes.
  • I have run dart analyze . in the terminal and addressed any issues.
  • I have run flutter test in the terminal and addressed any issues.
  • I have pulled from the stable branch before committing.
  • I have updated the CHANGELOG.md file with my changes (For the version, you can use X.X.X, I will later bump it after it's merged).
  • The PR title follows the conventional commit style (e.g., feat(paint-editor): add new triangle shape).
  • If this PR introduces breaking changes, I have verified that there is no way to implement the changes without them.

PR Checklist (Optional)

  • Tests have been added for the changes.
  • Documentation has been added/updated.

PR Type

Please indicate the types of changes this PR introduces by checking the relevant boxes:

  • Bugfix
  • Feature
  • Performance
  • Refactor (no functional or API changes)
  • Code style updates (e.g., formatting, local variables)
  • Documentation updates
  • CI-related changes
  • Other... Please describe:

Current Behavior

When the user finish action with a layer (drag or scale, etc), onLayerTapUp callback should be called, but currently isn't because handleTapUp checks for isLayerBeingTransformed. Please refer to Additional Information for my reasoning.

Issue Number: N/A

New Behavior

Remove isLayerBeingTransformed check, which seems to be unnecessary?

Breaking Changes?

Does this PR introduce a breaking change?

  • Yes
  • No

Additional Information

I did a bit of looking and found that handleTapUp has the isLayerBeingTransformed check. As of right now the only place where isLayerBeingTransformed becomes false is at _onScaleEnd. One problem is that _onScaleEnd is always called after handleTapUp, which means that currently there's no case that isLayerBeingTransformed would be true by the time the check takes place, which leads to onLayerTapUp never being called.

What lead me to think that this check is unnecessary is that the check also check for _isScaleInteractionActive and that isLayerBeingTransformed is only trigger on/off in the scale operation.

However, I'm not very confident that this is the fix so if this doesn't make sense, please feel free to close this PR

Copy link
Owner

@hm21 hm21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for creating the PR.

I'm not sure if removing state.isLayerBeingTransformed will cause any issues, since that check is mainly needed for multi-selection. However, there are no test failures, so I'll go ahead and merge it. But if any issues come up later, we might need to reintroduce that flag along with a widget test.

@hm21 hm21 merged commit 643dd79 into hm21:stable Dec 17, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants