chewie icon indicating copy to clipboard operation
chewie copied to clipboard

Fix zoomAndPan not having an effect

Open abalmagd opened this issue 7 months ago • 1 comments

  • Tests were made with version 1.11.3 example app

  • Closes #678

  1. Added missing values to copyWith method.
  2. Hide controls when interacting with zoom and pan.
  3. Move up the InteractiveViewer to the parent Stack.
  4. Increase example app android compile sdk version.
  5. Add zoomAndPan: true to example app controller.
  • After checks zoomAndPan works for both fullscreen and non fullscreen

abalmagd avatar Apr 30 '25 17:04 abalmagd

Codecov Report

Attention: Patch coverage is 43.47826% with 13 lines in your changes missing coverage. Please review.

Project coverage is 44.47%. Comparing base (e176704) to head (d51d4e5). Report is 10 commits behind head on master.

Files with missing lines Patch % Lines
lib/src/player_with_controls.dart 52.63% 9 Missing :warning:
lib/src/chewie_player.dart 0.00% 4 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #920      +/-   ##
==========================================
- Coverage   44.91%   44.47%   -0.44%     
==========================================
  Files          21       21              
  Lines        1574     1585      +11     
==========================================
- Hits          707      705       -2     
- Misses        867      880      +13     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Apr 30 '25 17:04 codecov[bot]

Any news from the team regarding this PR ?

@abalmagd : Maybe you could add the InteractiveViewer in the tree only if the zoomAndPan is set to true by saving the stack into a final var, then either returning that var or returning it embedded in the InteractiveViewer if the boolean is true. That way users that do not enable the zoomAndPan feature, will keep existing code unchanged.

maxbritto avatar Jun 30 '25 09:06 maxbritto

@maxbritto The TransformationController may still be passed and control the InteractiveViewer via the controller instead of gestures.

I added it to the tree if zoomAndPan is set to true OR if TransformationController is provided.

Also, I think zooming or panning via the controller does not require ui to be updated as it wasn't done with gestures, so the onInteractionUpdate and onInteractionEnd are unchanged.

abalmagd avatar Jun 30 '25 12:06 abalmagd

Nice change, hopefully this will help reviewers accept it 🤞

maxbritto avatar Jun 30 '25 13:06 maxbritto

@abalmagd please re-sync your changes with master.

Thanks

diegotori avatar Jun 30 '25 13:06 diegotori

Also, revert the formatting. Thanks.

diegotori avatar Jun 30 '25 13:06 diegotori

@diegotori Done, although most of the formatted/white space is due to inserting/replacing some parent widgets

abalmagd avatar Jun 30 '25 13:06 abalmagd

@abalmagd please re-sync your changes again with master.

Once that's done, then you should be able to keep the format. Otherwise, run dart format . on the library. Thanks.

diegotori avatar Jun 30 '25 15:06 diegotori

@diegotori Re-synced with master

abalmagd avatar Jun 30 '25 22:06 abalmagd

@abalmagd published in version 1.12.1. Thanks for your contribution.

diegotori avatar Jul 01 '25 01:07 diegotori