Fifourche commented 3 years ago

A PR to sync the slider with the active keyframe, so that if it is changed programmatically or via widget selection, the slider's value is changed accordingly !

Fifourche commented 3 years ago

Wow quick catch ! ^^ will do !

alisterburt commented 3 years ago

awesome! could you look into the test which is failing before we merge? https://github.com/napari/napari-animation/pull/93/checks?check_run_id=2692537547#step:9:233

Fifourche commented 3 years ago

Yes I'll have time to fix it soon ! :)

codecov[bot] commented 3 years ago

Codecov Report

Merging #93 (7c610b9) into main (ed848ea) will decrease coverage by 0.25%. The diff coverage is 76.78%.

@@            Coverage Diff             @@
##             main      #93      +/-   ##
==========================================
- Coverage   90.37%   90.11%   -0.26%     
==========================================
  Files          20       20              
  Lines         810      840      +30     
==========================================
+ Hits          732      757      +25     
- Misses         78       83       +5

Impacted Files	Coverage Δ
napari_animation/_qt/frame_widget.py	`86.11% <42.85%> (-4.80%)`	:arrow_down:
napari_animation/animation.py	`78.82% <75.00%> (+1.90%)`	:arrow_up:
napari_animation/_qt/animation_widget.py	`87.91% <87.50%> (-0.83%)`	:arrow_down:
napari_animation/_tests/test_animation.py	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ed848ea...7c610b9. Read the comment docs.

Fifourche commented 3 years ago

I commited some changes - too many at once probably ! I'll avoid that in the future, for the git history to be more readable.

In here I removed the current_key_frame attribute, and the references to key_frames.selection._current to rely only on key_frames.selection.active ; as we discussed it elsewhere, only one of them would be needed and keyframe.selection.active corresponds to the selected keyframe in GUI !
Fixed the slider_moved callback issue.
If no frame is selected (active), errors are raised when attempting deletion or replacement, and delete button is disabled.

Fifourche commented 3 years ago

Done ! Thanks again for the help and the good vibes :)

alisterburt commented 3 years ago

great working with you as always!

napari / napari-animation

Sync slider with active keyframe #93

Codecov Report