mirror of
https://github.com/NickeManarin/ScreenToGif.git
synced 2026-04-25 15:15:51 +03:00
[GH-ISSUE #881] Playback buttons are disabled after "Save as" #685
Labels
No labels
copy cats
duplicated
future feature
pull-request
⬜ Accepted
⬜ Completed
⬜ Help Wanted 💪
⬜ In Progress
⬜ Missing Details
⬜ Pending
⬜ Waiting For Answer ⏳
🆕 feature preview
🔷 Bug 🐛
🔷 Out Of Scope
🔷 Out Of Scope
🔷 Question
🔷Enhancement
🔷Enhancement
🔷Invalid / External
🔷Knowledge Base
🔷Won't Fix
🕑 High
🕑 High
🕑 High
🕕 Medium
🕙 Low
🕛 Critical
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/ScreenToGif#685
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @pawlos on GitHub (May 6, 2021).
Original GitHub issue: https://github.com/NickeManarin/ScreenToGif/issues/881
Originally assigned to: @pawlos on GitHub.
I've noticed a weird behavior of the editor. After saving an animation all of the Replay buttons are disabled (they were enabled before ofc).
Did some checking and the responsible for this is the
_applyAction == nullin the following codeas when Save is executed the
_applyActionis set to_applyAction = SaveAsButton_Click;in thefinallyofSaveAsButton_Click. After that apparently it never gets nullified again. I guess this is done to prevent playing the video while saving/encoding is performed (although my simple/quick test with commenting out this line didn't show any issues of playing while saving).I guess the fix would be to set
_applyActiontonullwhen encoding is done, but not sure if that's possible in the current architecture of the system?@pawlos commented on GitHub (May 9, 2021):
@NickeManarin I could work on a fix for this, but as mentioned not sure if there's a clean way to do this with the current architecture of the app. Don't see it's doable without rewriting the big chunk of code. Any guidelines how you would try to solve this?
@NickeManarin commented on GitHub (May 9, 2021):
The
_applyActionshould be set to null when the side panels are closed.Like this, I believe:
I want to rework the Editor, with a new architecture in mind.
The current codebase is slow and hard to change.
Some of the behaviors (like this one that we are discussing, for example) made a bit of sense back then, but I believe that it can be removed with little adverse consequences.
And yes, this
_applyActionis being used in aCanExecuteto prevent the playback of the preview when a panel is open. But the primary use is to allow the user to press Apply/Save.@pawlos commented on GitHub (May 10, 2021):
Thanks for the info. I think it wont be that easy as
ClosePanel(false, true);as in this caseClosePanel(false, true);is called before setting the_applyActionin the finally block. But I'll look around more anyway.Taking into the consideration the following
I want to rework the Editor, with a new architecture in mind.should I not try to fix it in the current arch? Or the new one is long in the future?@NickeManarin commented on GitHub (May 11, 2021):
I'm planning a redesign for the editor for v3.
I think that it's going to take a while for me to finish it.
Ahh, sorry. I didn't see that.
@pawlos commented on GitHub (May 12, 2021):
@NickeManarin I've created a PR. It's not the fix I was expecting to do, but I went through the code and I think this check for
_applyActionin Playback was unnecessary in the first place (maybe before it looked differently). Did the testing and I don't see any unwanted behavior w/o this check. The playback is disabled as long as the side panel is opened and get enabled when it's being closed.Please feel free to comment if you see something's not being right.
@NickeManarin commented on GitHub (May 15, 2021):
Thanks. I merged the PR.
I could not find any other issue with it too.