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

AppSettingsView: don't block settings when DND is active #102

Merged
merged 1 commit into from
Mar 13, 2024

Conversation

danirabbit
Copy link
Member

@danirabbit danirabbit commented Mar 9, 2024

Fixes #91
Prevents blocking settings when DND is active

Screenshot from 2024-03-09 13 32 51

I know InfoBar is deprecated but we use it a lot so I think we probably will need a Granite replacement so I'm using it assuming we'll fix all infobars at once later.

@danirabbit danirabbit requested a review from a team March 10, 2024 20:54
@jeremypw
Copy link
Collaborator

This issue does not seem to affect main on elementaryos 8?
Screenshot from 2024-03-12 15 44 52

@danirabbit
Copy link
Member Author

@jeremypw when you first open settings, try resizing vertically. Then open this pane and see that it forces the window height larger in main

@jeremypw
Copy link
Collaborator

jeremypw commented Mar 12, 2024

@danirabbit Ah, OK - I see the issue now. I'll review again.

It also happens with several other plugs if you make the initial height small enough. Maybe there should be a reasonable minimum height of the main window, which all the plugs can fit in? At the moment it can be made nearly the same height as the headerbar.

@danirabbit
Copy link
Member Author

I'm not sure what that value should be but I'm not opposed to adding a minimum window height 👍

Copy link
Collaborator

@jeremypw jeremypw left a comment

Choose a reason for hiding this comment

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

I got some horrible visual glitching, when dragging the window at (fairly) small size.

Screenshot from 2024-03-12 19 44 51

@danirabbit
Copy link
Member Author

@jeremypw I saw that too. I don't think there's anything I can do here about that since it seems like a GTK bug

@danirabbit
Copy link
Member Author

@jeremypw looks like this also happens in Network settings

@jeremypw
Copy link
Collaborator

Yes, I now see it happening main and some other plugs (but not all). I guess its not a blocker for this PR then. Hope it is fixed upstream soon!

@jeremypw jeremypw dismissed their stale review March 13, 2024 19:20

Upstream issue

Copy link
Collaborator

@jeremypw jeremypw left a comment

Choose a reason for hiding this comment

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

Code looks good and works well.

@jeremypw jeremypw merged commit 1dde134 into main Mar 13, 2024
3 of 4 checks passed
@jeremypw jeremypw deleted the danirabbit/appsettingsview-dndinfobar branch March 13, 2024 19:21
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.

Forces minimum window height
2 participants