Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch revealer transition fixing re-appear #174

Closed
wants to merge 2 commits into from

Conversation

peteruithoven
Copy link
Collaborator

Fixes: elementary/wingpanel-indicator-bluetooth#64

After the bluetooth indicator disappears it doesn't properly re-appear.
This can be reproduced by running:

systemctl restart bluetooth

It seems to have something to do with the RevealerTransitionType.SLIDE_LEFT transition of the Gtk.Reveal that's used for every indicator.

If I add an exception to the following list changing the Reveal transition to anything except SLIDE_LEFT or SLIDE_RIGHT there is no issue.
https://github.com/elementary/wingpanel/blob/master/src/Widgets/Panel.vala#L232-L251

If I switch the transition_type from right to left when the visibility of the indicator is toggled it also re-appears normally. Which is what I'm doing with this PR.

@worldofpeace
Copy link

I've tested this in elementary OS Juno and other platforms and I cannot reproduce the aforementioned issue ❇️

@peteruithoven
Copy link
Collaborator Author

peteruithoven commented Nov 1, 2018

I have to admit that I can't explain why this is an issue.

While creating a simple test / example I ran into another issue:
https://stackoverflow.com/questions/53093688/gtk-revealer-slide-right-slide-left-issue
But that's probably not relevant to this issue.
When the orientation of the box in my example is set to Horizontal I can toggle the revealer just fine, without needing to switch the transition type.
slide-right-orientation-switching

@jeremypw
Copy link
Collaborator

jeremypw commented Nov 9, 2018

This seems like a bit of a hack to me. Probably best to identify the cause more precisely. Maybe a race? I could not reproduce on my dev install of Juno - I'll try it on my vanilla install.

No, could not reproduce on vanilla Juno either.

@jeremypw
Copy link
Collaborator

jeremypw commented Nov 9, 2018

Maybe throttle calls to set_icon or update_icon in the bluetooth indicator so that it doesnt called in the middle of a reveal?

@worldofpeace
Copy link

I could not reproduce on a vanilla Juno install either, until it was updated.

@peteruithoven
Copy link
Collaborator Author

peteruithoven commented Nov 9, 2018

@worldofpeace

I could not reproduce on a vanilla Juno install either, until it was updated.

"Untill"? So after updates you can?

@worldofpeace
Copy link

@peteruithoven Correct. After updates I can reproduce.

Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peteruithoven Would it be better to set the orientation of the container then or maybe switch the container type? I agree with @jeremypw that this seems kind of hacky

@peteruithoven
Copy link
Collaborator Author

@danrabbit which container though...

This is the situation:

  • IndicatorMenuBar (Gtk.MenuBar)
    • IndicatorEntry (Gtk.MenuItem)
      • Gtk.Revealer
        • base_indicator.get_display_widget() (Gtk.Widget)

None of these have an orientation. I've tried setting the MenuBar.pack_direction explicitly to Gtk.PackDirection.LTR but that doesn't seem to help.

I've noticed I can also reproduce the issue by rapitly switching nightlight on off. You see it sometimes not showing up or even showing only half of the icon:
screen record from 2019-01-11 21 20 15
screen record from 2019-01-11 21 19 21
(Using Enter to switch the button helps)

@jeremypw I'll look into icon updates.

@peteruithoven
Copy link
Collaborator Author

@jeremypw the nightlight icon isn't actually updated when you enable/disable Night Light, that only impacts visibility (visible). So in the case of the Night Light indicator it can't be icon updates.

@peteruithoven
Copy link
Collaborator Author

peteruithoven commented Jan 11, 2019

Looks like commenting the delayed re-ordering fixes the issue:
https://github.com/elementary/wingpanel/blob/master/src/Widgets/IndicatorEntry.vala#L82-L85

Removing the previous timeout and increasing the interval helps a great deal. But that means another arbitrary value, which could still give issues on slower systems and adds a delayed sudden jump in the indicators.

Would be great if we could do something like: #155

@peteruithoven
Copy link
Collaborator Author

Superseded by: #187

@peteruithoven peteruithoven deleted the fix-re-appear2 branch February 25, 2019 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants