Skip to content

Conversation

@danirabbit
Copy link
Member

@danirabbit danirabbit commented Nov 5, 2021

No description provided.

@danirabbit danirabbit changed the title Basic setup for settings sidebar Stack Switcher → SettingsSidebar Nov 5, 2021
@hanaral
Copy link

hanaral commented Dec 19, 2021

Something about this layout seems off, maybe it's that the current one is more natural to read (I think because it's above the page and horizontally laid out)

@treppenwitz03 treppenwitz03 mentioned this pull request Jun 10, 2022
9 tasks
Jeremy Wootten added 2 commits June 30, 2022 19:17
@jeremypw
Copy link
Collaborator

jeremypw commented Jul 3, 2022

@danrabbit I have fixed conflicts with master. One problem I have noticed with using Granite.SimpleSettingsPage (a scrolled window) with Multi-tasking View is that if the window height is such as to require scrolling then if you scroll on the content area you will at some point enter a combo and start changing the options instead of scrolling - which is definitely undesirable. The simplest solution is to set a minimum height on the Settings window sufficient that scrolling is never required on Multitasking View.

@jeremypw
Copy link
Collaborator

jeremypw commented Jul 3, 2022

I have proposed a change to Switchboard (elementary/settings#242) so that this view does not have to scroll and all the settings are visible when it is opened.

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.

The code looks good and the change makes sense in the context of a strategy of moving all the plugs using a Stack to a SettingsSidebar (as many are already).

The screenshots need updating.

@danirabbit danirabbit marked this pull request as ready for review April 11, 2023 00:34
@danirabbit danirabbit requested a review from jeremypw April 11, 2023 00:34
@danirabbit danirabbit mentioned this pull request May 1, 2023
Copy link
Member

@lenemter lenemter left a comment

Choose a reason for hiding this comment

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

I get warnings when I close the System Settings app in this branch:

** (io.elementary.switchboard:4237): CRITICAL **: 20:14:27.643: granite_settings_sidebar_row_get_page: assertion 'self != NULL' failed

(io.elementary.switchboard:4237): Gtk-CRITICAL **: 20:14:27.643: gtk_stack_set_visible_child: assertion 'GTK_IS_WIDGET (child)' failed

Overall I think this is a step in a good direction but it needs more polishing. For example:

  1. 'Wallpaper' and 'Dock & Panel' pages shouldn't use the same icon;
  2. Appearance tab looks weird compared to other pages because of it's long description while other pages don't have it at all;
  3. IMO It would be nice if every page's max width was the same and these header labels wouldn't jump around

if (((GLib.DBusProxy) pantheon_act).get_cached_property ("PrefersColorScheme") != null) {
grid.attach (dark_label, 0, 0, 2);
grid.attach (dark_info, 0, 1, 2);
grid.attach (prefer_style_box, 0, 2, 2);
Copy link
Member

Choose a reason for hiding this comment

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

This grid's attaches should start with 0, 0, not from 0, 2 straightaway.

@danirabbit danirabbit marked this pull request as draft July 20, 2023 17:39
@danirabbit danirabbit closed this May 18, 2024
@danirabbit danirabbit deleted the settings-sidebar branch May 18, 2024 15:50
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.

5 participants