Automatic Transparency #513
Conversation
|
@micheleg did you get a chance to try this? I'm travelling at the moment, but I can look at this again later this week. |
|
Haven't tried it yet. I have been trying Gnome 3.25.x and I have to say I don't seem to particularly enjoy that feature on the panel, however I think it makes sense to try to be consistent. I'll try to have a look at it. Before this I'd like to try to merge the previews on click branch |
|
Sounds good. Let me know if there is any remaing to-do on the previews PR. |
franglais125
force-pushed
the
transparency
branch
5 times, most recently
from
3f905e2
to
969f889
Compare
|
Branch updated. It now includes a checkbutton, and the dynamic transparency uses the alpha value selected by the user. Very similar to #62 .
|
franglais125
force-pushed
the
transparency
branch
from
ce4e6b2
to
e2dffb7
Compare
franglais125
force-pushed
the
transparency
branch
from
381c6d1
to
df38b93
Compare
|
I did test your branch and quickly gloss over your changes. They look good to me, but I'm not that familiar with the code itself :) However, I have a suggestion: with this dynamic transparency, it removes any possiblity to have the launcher (especially in intellihide mode) not fully opaque once a window touches it. |
|
The second idea (I'm happy to contribute btw, as I think we'll need to cherry-pick this soon as well for ubuntu-dock) is to only apply this settings in non intellihide mode. |
|
@didrocks thanks a lot for the feedback! I keep forgetting to test intellihide when I submit PRs, as it's not something I use regularly... Before trying to implement one of your ideas, let me try to see if I can fix the issue at its root: reacting to intellihide animations in the first place. I included a commit to address this, I'm not claiming it is fixed, but it is perhaps better. What do you think? [ce76087]. |
franglais125
force-pushed
the
transparency
branch
from
ce76087
to
792340a
Compare
|
I'm still a bit confused by how this should behave. I'm personally distracted by having the top panel and the dash background changing transparency, expecially independently, while moving windows around.... If I have undertood the issue and correctly interpreted @didrocks suggestion, I think that:
|
I was thinking a bit about this too. The problem I saw with having the dock being opaque when in intellihide mode is from the user's perspective: if intellihide is on, toggling dyamic transparency in the GUI would have no effect, which would be weird/seen as a bug. Alternatively, this setting could be greyed out if intellihide is on, but then it is again weird to guess why it is like that. In the end, I was happy with the latest fix I included, as you said. In any case, I'm waiting for @didrocks 's feedback again, as I'm not sure I fixed what he pointed out. I certainly don't intend to be dismissive of the suggestions. |
|
Thanks to both of you! I tried latest branch and indeed, the Intellihide glitch is now fixed. I'm quite in favor for michele's suggestions, being:
Does it sum up and align with your understanding? I'm happy to give any help if needed :) |
The first problem I found was that we need to adjust the property Specifically, I guess this is not a problem for "Ubuntu Dock", as I'm assuming it will be packaged with a specific theme anyway. But this wouldn't be the case for anyone installing d-t-d from e.g.o/git. In any case, I'm not exactly experienced with css, so please let me know how to work around this. I feel like I'm missing something! CSS reference: https://developer.gnome.org/gtk3/stable/chap-css-properties.html
|
|
Those are valid points. I'll let Michele answer if the css or the extra settings path is the better with what he has in his mind. I'm starting to think (seeing how the code is structured) that another opacity settings for the start of the transparency transition makes sense. On intellihide, indeed, it's true that it can look weird when no window touching it or the top panel. So, you may be right by keeping the min and max transparency to accomodate. However, I think that the "max" value (when hidden) shouldn't be opaque, but respecting the same opacity settings (which would be a little bit transparent in ubuntu dock when on top of a window pushing the dock away). |
|
Quick comments:
I didn't realise that. I don't think we can easily workaround it. I would still offer two css classes for our "embedded theme" and third parties themes (if they want to support dash-to-dock). My idea is 1) we support the defualt theme and if possible adapt to other themes. 2) We provide great flexibility for to custom themes to define their own style. 3) The user can still manually tweak some parameter
I still think all Ubuntu-dock needs should be satisfiable with changes at the theme level (with us adding the above mentioned css classes). On the dash-to-dock side, it's not too much of a deal to add another opacity slider if that is the most logical sulution. For the intellihide case, I'm not sure what's the best visually speaking (I haven't used 3.26 enough to judge). Everything looks a bit unpleasnt to me with a dock touching the top panel (I remember the gradient mockups, but it seems to still be just transparent as of now). I'm even wondering if the Ubuntu folks have even considered disabling the dynamic transparency... As we seems to have a working implementation, and if we a re going to add the second opacity settings, I would go for the current behaviour but with the opacity values corrected (no full opacity). |
docking.js
Outdated
| @@ -350,7 +350,7 @@ const DockedDash = new Lang.Class({ | |||
| ]); | |||
|
|
|||
| this._injectionsHandler = new Utils.InjectionsHandler(); | |||
| this._themeManager = new Theming.ThemeManager(this._settings, this.actor, this.dash); | |||
| this._themeManager = new Theming.ThemeManager(this._settings, this.actor, this.dash, this); | |||
There was a problem hiding this comment.
Could be just:
this._themeManager = new Theming.ThemeManager(this._settings, this);
There was a problem hiding this comment.
Thanks. Included this here [e0fc16a].
We did, but for obvious reasons, we don't really want to deviate from upstream's design on that one. I think we can achieve something matching with a very transparent dock when no window is touching them.
Yeah, so basically: current behavior, 2 opacity settings (min/max), intellihide mode respecting as well those 2 opacity settings. That makes sense to me as well. Tell me if you need any help. |
|
Thank you both again for the feedback :)! I was taking some time before tackling the code, as I wanted to make sure I understood what we want. I'll try to come up with a few patches in the next few days. Further feedback is of course appreciated! |
|
Introduced three commits:
If you look at the new style classes added, you'll see that the I still need to rework the GUI+settings to add the second slider. To do:
|
|
One last thing while thinking about it and playing with it a little bit more: I think, for full height panel (like ubuntu-dock default and your special settings for it), I think it will make sense to just link the transparency detection with the top panel one. You can consider it as just one UI and not 2 independant part, which could look "flaky". That could be one option (that I'll toggle in ubuntu dock IMHO), which is "link panel and dock opacity state" or something along those lines. Only the Shell (via the top panel transparency decision) would in that option decides if toggling something transparent or not. |
|
@micheleg Good point. I run Debian 9 too, so whenever I test some new code, the first thing I use is 3.22. This at least ensures we are compatible with Gnome = 3.22. I never test something older though... Perhaps I should, every once in a while. As for this branch, while it is compatible with Gnome <= 3.24, what happens is that (This is why I test for If you are ok with this, to-do (feel free to extend it, also @didrocks if you have further comments!):
|
|
Great! For now not crashing or thowing errors in 3.22+ is ok for me, which I plan on support from now on. Let's finalise the bahaviour for 3.26 (which is also what ubuntu dock will support), and than before releasing a new version of dash-to-dock we can clean up the settings, in case hiding the unnecessary ones. |
|
@micheleg I hope I addresses all/most of your comments. In the case of an eventual merge, the new commits can certainly be squashed where appropriate. Let me know if you want me to do this, just o save you some time. I didn't do so to make clear what the new changes are. |
|
Thanks for working on this over this week-end! (And late, from what I see). I'm going to give another round of testing of the new code. The one from Friday was working very well with 3.26, I didn't notice any issue. I'll try to the updated code today. I guess it's going to be too late for uploading today, I'm trying to aim for tomorrow/wednesday thus (which should be ok given the amount of testing). How should we proceed? I see you are ok maybe to merge this and refine later for earlier G-S release. Note that I will only cherry-pick the commits + some other fixes from master for 17.10: I can't introduce the Unity backlight supports as we are past Feature Freeze (this is why I didn't rebase the branch recenly) and will get the FF exception for this dynamic opacity only. I'll wait for next cycle thus to do a full rebase, so if you prefer me to cherry-pick directly from this PR branch and not merge it directly, it's fine as well. |
|
Ok, just gave it an advanced round of testing and didn't spot anything weird or incoherent in the dock behavior. Even with intellihide, it's nice to see the transition starting when you push it on side (as it will reappear with max opacity)! A little gotcha, which doesn't really concern the ubuntu dock case: when you change the advanced settings for min and max opacity in the adaptative or dynamic case like the switch on/off the customize switch or play with the switchers:
Excellent work :) |
|
@didrocks Thanks a lot for the testing!
I'll try to see if I can work around these two annoyances. Lower priority though! @micheleg I checked the compatibility of arrow functions. They were apparently introduced in libmozjs = 24, which was included as far back as Jessie. I tested the new code on 3.18 and it worked fine. I actually thought 3.18 was old, but as an example Ubuntu 16.04 is using it. Anyway, I'll try to be more careful with coding style and JS functionality! |
|
Good! Let me see see If I can clean up the branch and merge it in master. |
|
@micheleg thanks! Let me know if you need some help. |
theming.js
Outdated
| this._borderAlpha = borderAlpha.toString(); | ||
| this._originalBorderAlpha = originalBorderAlpha.toString(); | ||
| this._updateStyles(); | ||
| _getAlphas: function() { |
There was a problem hiding this comment.
Do we need this only for the styles defined in our stylesheet? Or is is necessary also to support third party themes?
There was a problem hiding this comment.
We need this for both:
- to retrieve our own
opaqueandtransparentstyles - if a (3rd party) theme author includes his own
opaqueandtransparentstyles
Does it answer your question?
There was a problem hiding this comment.
for third party themes, we should probably just apply the css classes, no need of retrieving the opcaty values, or am I missing something?
There was a problem hiding this comment.
The css classes might come with specific colors, and we might be overriding this already.
This was the whole problem since the beginning: is there a property e.g. alpha: 0.8 that we can use? If not, we need the full rgba().
There was a problem hiding this comment.
If the color comes from the theme it's fine, that's what the auhor wants. Doesn't it makes sense? I can try to preapare a patch and see if it work, as I might be missing something :)
There was a problem hiding this comment.
What I meant is, if the user sets a specific color in the settings, this color will be overridden by the theme.
Normally, even if the theme proposes a default color, the GUI lets the user set whatever color he wants.
There was a problem hiding this comment.
In any case, as you suggested elsewhere, we could just drop the whole opaque/transparent styles, and remove this machinery. I'm fine with that!
stylesheet.css
Outdated
| @@ -95,6 +95,20 @@ | |||
| background: #2e3436; | |||
| } | |||
|
|
|||
| /* Only alpha value is used */ | |||
| #dashtodockContainer.opaque { | |||
There was a problem hiding this comment.
I know that I suggested to define these styles here, but I don't see the point of this any more.
There was a problem hiding this comment.
Instead you would simply set min/max alpha values in the settings for instance?
I think the idea was to better integrate to specific themes, in the case a theme author wants to set a specific opacity level for the panel and the dock (both opaque and transparent).
I don't feel strongly about this though.
There was a problem hiding this comment.
This is connected with the idea of appplying classes directly. The issue originates from my idea of defining these classes in our theme in the first place. If we don't do it (or better we do it only for the "our custom theme" (dashtodock class), we do not overwrite anymore the style and we can just apply the classes. I'm mainly trying to avoid this weird workaround of adding an actor to retrieve the style
There was a problem hiding this comment.
This https://github.com/micheleg/dash-to-dock/tree/experimental_tweak_transparency_behaviour is wip towards what I was trying to describe:
- With custom theme enabled, integrate as well as possible with the default theme
- Enable adaptive transparency if the dock is in panel mode and vertically placed, matching the background of the panel.
- Keep the dock opaque in the other cases (personal preference)
- For custom themes, in the DEFAULT behavior, the style provided by the theme will be used. Using the .solid class, themes writers can directly support dash-to-dock.
- If this still does not match, the user can play with the advanced settings.
There was a problem hiding this comment.
Ah, I see! Indeed, I was not understanding what you meant. I'll give it some testing and report back. So far the only problem I found is that some styles are completely black (I guess due to my using (0,0,0,alpha) for rgba in the stylesheet).
I love the changes so far, I didn't know you wanted to change the default behavior as well :).
There was a problem hiding this comment.
@micheleg I included a few fixes here: https://github.com/franglais125/dash-to-dock/tree/experimental_tweak_transparency_behaviour
theming.js
Outdated
| @@ -360,7 +360,8 @@ const Transparency = new Lang.Class({ | |||
|
|
|||
| // Window signals | |||
| global.get_window_actors().forEach(function(win) { | |||
| this._addWindowSignals(win); | |||
| if (win.get_meta_window().get_wm_class() !== 'Gnome-shell') | |||
There was a problem hiding this comment.
This works upstream because they never disconnect the signal? How do you reproduce the bug?
There was a problem hiding this comment.
Exactly, it's a bit annoying, but sometimes I can trigger this. I tried to pinpoint the root cause of this, but I was unable to. There are some actors that don't get properly connected, and then fail disconnection. As you guessed, the panel signals are never disconnected and hence no warning appears.
There was a problem hiding this comment.
Ok: we have the same problem in the intellihide class https://github.com/micheleg/dash-to-dock/blob/master/intellihide.js#L107-L109
There was a problem hiding this comment.
I don't use intellihide, so perhaps this is why I didn'd encounter the error before. Did you see it anywhere else? Like so:
gnome-shell[1313]: /build/glib2.0-B1uXKV/glib2.0-2.50.3/./gobject/gsignal.c:2641: instance '0x55fc2d1d47f0' has no handler with id '52060'
where 0x55fc2d1d47f0 is a window instance (I did check this in the past).
|
@didrocks: I have prepared a slightly cleaned-up branch in the development branch (https://github.com/micheleg/dash-to-dock/commits/development). It is substantially the same of this PR, beside very minor cosmetic changes and history cleanup. This is what I'm going to be merge in master, although I still want to improve the behaviour on dash-to-dock side. I think the best strategy for you is to cherry pick from there. |
|
Here we go! I just based the ubuntu-dock branch, cherry-picking the needed commits. Thanks a lot @micheleg and @franglais125 for working on this. After the currrent finale round of testing, everything looks good to me. I'm planning as well blogging about your excellent work there :) Do not hesitate if you need anything! |
|
@didrocks thanks a lot for the feedback and ideas! Good luck with the FF exception! |
|
So I've merged this in master as of [225b51d]. |
|
@micheleg Thanks a lot! Let me know if you find remaining issues or things to polish. One quickly comes to mind: hide the "Adaptive" option on Gnome < 3.26, or at least offer an explanation on the UI. |
|
That's exactly what is missing on our side, before reconsidering our default behaviour. I would possibly hide the adaptive case. For adding explanationas, I don't know where to put it. An idea would be to update the "subtitle" of the option, the one that now "Tune the dash backgrond opacity". |
franglais125
force-pushed
the
transparency
branch
2 times, most recently
from
1421b49
to
93826df
Compare
|
@micheleg I added two commits to see how this would work. In general, I'm not really satisfied with the result. I needed to work around a few things which I didn't expect:
|
This variable is accessed from theming.js.
franglais125
force-pushed
the
transparency
branch
from
93826df
to
a520105
Compare
|
Added one commit to remove JS warning. |
|
Last mentioned commit was merged as ae7b86e. |
|
Finally closing this PR. Thanks everyone for the effort! |
|
Thanks again for your work! :) |
|
Hi all, sorry to barge in. I can't find the options mocked here in the version available at https://extensions.gnome.org/extension/307/dash-to-dock/. How can I get the adaptative behaviour? (Got version 61 today.) |
|
v62 is still awaiting review (#679). You have to manually install the latest release if you want access to those features. |
Here is a proposal to imitate the work done upstream on the top bar. The idea is that the dash blends in more uniformly, by behaving according to the top bar's new transparency when 'free-floating'.
Default behavior
As of Gnome 3.26, the top bar will be only 20% opaque by default. Hence, as a proof-of-concept, I have modified the default behavior for the dash. I have lowered the default opacity from 80% to 20% (as upstream), and the transparency logic is now a bit different. So far we assumed that the dash was opaque, unless the opacity setting was used.
I have now inverted this logic: the dash is 20% opaque, unless the user decides to set the opacity (permanently) to something else.
Option
I did it this way for simplicity, but we can of course change how this is done. The setting could be a combo-box with a few options like "Default, Automatic, Permanent" or something like that.
Regarding the code
I tried to keep the
_updateSolidStyle()function as close as possible to upstream, so that it's easier to update if something changes. There are of course some differences stemming from the dash's complexity:Also, I tried to limit the number of operations on this function, as it gets called very often, hence having an impact on CPU time and responsiveness.