pop-os / cosmic-dock

Pop!_OS fork of https://github.com/micheleg/dash-to-dock/tree/ubuntu-dock
GNU General Public License v2.0
72 stars 18 forks source link

fix: during initialization, set animation time to 0 #87

Closed pbui closed 2 years ago

pbui commented 3 years ago

If manual hide is enabled, then we do not want to animate the dock at all. However, the _animateOut function performs some side-effects on completion, so we need to run it... therefore just set the timeout to 0.

To distinguish between normal behavior and initialization, introduce a boolean init variable to _updateDashMode and _updateDashVisibility that by default is set to false and thus performs the old behavior. In _initialize, however, we call _updateDashMode with this parameter to true and thus eliminate the animation time.

The outcome of this change is that unlocking the screen or reloading the extension will not animate the dock if it is set to manual hide.

Fixes #47 and pop-os/cosmic:#145

pbui commented 3 years ago

Hmm. Ok, I can reproduce that behavior if the Dock Visibility is set to Always Visible. If you change it to the other two options, it will not have the resizing problem.

I could try to fix this in the extension (if manual_hide is active, then also change the visibility), but it might be better for the desktop-widget to change the visibilty when the user disables the Dock.

What do you think is the better solution?

pbui commented 3 years ago

Note, I have Dock Visibility set to Intelligently Hide, which is why I missed this.

jacobgkau commented 3 years ago

Ah, I see. Unlocking works as expected with the other two visibility modes.

I could try to fix this in the extension (if manual_hide is active, then also change the visibility), but it might be better for the desktop-widget to change the visibilty when the user disables the Dock.

If "disabling the dock" is actually just enabling a manual_hide setting, then having desktop-widget change the visibility when disabling makes sense to me on the surface.

However, desktop-widget would then need to keep track of the old visibility setting and restore it when the dock is re-enabled, or else the visibility would be reset to either Always hide or Intelligently hide (neither of which is the default option) when it's re-enabled. Having the visibility change just by flipping the Enable Dock switch off and on would not be desirable; a user wouldn't expect that to persistently affect other settings, and it might look like the dock wasn't re-enabled as requested if it's then hidden.

Also, addressing this in desktop-widget's handling of changing the settings would not fix the visual glitch for users who already have Enable Dock flipped to off and Dock Visibility still set to Always visible. For this to be a proper solution, it would need to apply retroactively with the package update, or at least with the next release upgrade.

With those two points in mind, it seems like addressing this fully in cosmic-dock would be better than relying on desktop-widget at this point in time.

pbui commented 3 years ago

@jacobgkau I found a simple fix. This should prevent the dock from being visible if manualhide is one. I no longer get the resizing behavior when the visibility is set to Always.

Please let me know if this fixes it for you.

pbui commented 3 years ago

Arg... sorry, had some issues git amending... it should be up-to-date now.

jacobgkau commented 3 years ago

On 1503032, the glitch is now gone when the dock is disabled. However, when re-enabling the dock, windows don't resize to get out of the way:

Screenshot from 2021-11-29 15-00-46

If I change visibility to either of the hide options and then back to Always visible, then the windows do get out of the way, and they continue to do so when toggling Enable Dock back and forth. I can recreate it again by disabling the dock while set to Always visible, restarting the shell, and enabling it again.

pbui commented 3 years ago

Hmm... Ok. I can reproduce that behavior as well. Let me think on this a bit more and I'll see if I can find away around it.

Thanks for the feedback.

pbui commented 3 years ago

@jacobgkau Ok, with the latest commits you should able to toggle the dock on and off with proper resizing behavior while fixing the main issue of keeping the dock hidden on screen unlock.

I incorporated some refactoring from upstream in regards to "dock-fixed" and then used the new functions to properly implement the manualhide toggle.

I did two commits to logically separate the changes, but I can rebase them into one commit if you wish.

Let me know if you run into any other problematic test cases.

jacobgkau commented 3 years ago

I'm currently seeing the dock show up if it's set to Intelligently hide and disabled (when I move the cursor to the bottom of the screen):

Screenshot from 2021-11-30 10-31-53

jacobgkau commented 3 years ago

Since this is taking some back-and-forth to get right (which is perfectly fine), I wanted to point out that master_impish has already been branched off for 21.10, which is very near release. It may be a better use of effort to solve the issues for that branch instead of master_hirsute; otherwise, someone will need to apply the improvements to master_impish separately once they're finished here.

master_impish seems to have the opposite problem from what this is solving, where the dock actually animates in on every unlock when it's enabled rather than animating out when it's disabled.

pbui commented 3 years ago

Ok, I think the latest commit should fix it... I didn't really understand what the .bind() method was doing and after reading more about it, I can see now that it doesn't actually call the method (it mades a new one). Now that we actually the method, the settings are updated and the Dock should behavior properly.

pbui commented 3 years ago

Regarding master_impish, I'd like for this to be resolved in master_hirsute first since that is what I am using.

That said, if I can run the master_impish branch on 21.04 (I do not want to use the beta yet), I'd be happy to look at that issue after this is resolved. Otherwise, I may be able to try out the beta in two weeks and then look at the issues present in the new branch.

jacobgkau commented 3 years ago

On fd93117, when the dock is set to Intelligently hide and enabled, after I change the dock's position (bottom/left/right), I can't make it show with the cursor until I move between workspaces at least once.

pbui commented 3 years ago

Hmm... this only happens when "extend dock to the edges of the screen" is enabled... which I typically don't have.

If this is disabled, then it behaves properly. I'll have to think about this some more.

pbui commented 3 years ago

@jacobgkau Ok... I think I've fixed the issue with changing the dock's position (while keeping all the other edge cases working). To do this, I only set the animation time to 0 for the manualhide mode during initialization. 🤞

jacobgkau commented 3 years ago

With the dock enabled but set to Always hide or Intelligently hide, the dock briefly appears when unlocking the screen before it's hidden. This visual glitch does not occur on the currently-released version.

pbui commented 3 years ago

I noticed that... I didn't remember if that was expected behavior or not. I'll take another look.

pbui commented 3 years ago

The latest commit 5023582 separates animation in time from animation out time. During initialization, always set the animation out time to 0. This appears to resolve the last problem of the dock appearing in Always hide or Intelligently hide mode during unlocking.

pbui commented 3 years ago

@jacobgkau Thanks for your patience and testing!