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

feat: libadwaita preferences #229

Merged
merged 1 commit into from
May 31, 2023
Merged

Conversation

bennypowers
Copy link
Contributor

@bennypowers bennypowers commented May 28, 2023

Closes #173
Closes #217

This draft PR migrates various preferences widgets to libadwaita

So far, I've just done the color widgets inside the color panel.
If maintainers are interested, my plan is to work on the "leaf nodes" first - the individual pref widgets, and when those are all done, move on to the views, using tab pages and other adw containers

image

@bennypowers bennypowers force-pushed the feat/prefs-adw branch 2 times, most recently from 9fc3896 to 0af8d47 Compare May 28, 2023 19:27
@jmmaranan
Copy link
Collaborator

Hi @bennypowers, thank you for incoming the contribution. I would agree preferences needed the makeover. I can't wait to try it out

@bennypowers
Copy link
Contributor Author

bennypowers commented May 29, 2023

I've migrated most pages to adw
image
this removes many lines of code, which is nice

still to do

  • figure out why most of the icons don't work
  • fix the tabbed/stacked dropdown
  • implement the keyboard page
  • i might refactor colours and other prefs to connect to the settings schema one-to-one, and use .ui files for the prefs
  • i might implement quick settings

@bennypowers bennypowers changed the title feat: Preferences using libadwaita widgets feat: libadwaita preferences May 29, 2023
@bennypowers
Copy link
Contributor Author

bennypowers commented May 29, 2023

As of 526e0f7, all the prefs pages are implemented in terms of adwaita concepts and widgets.

Somewhere along the line, I think I broke the schemas, since dconf tells me I have nothing at dconf list /org/gnome/shell/extensions/forge/keybindings/, so that will have to be fixed before merging.

As well, I still would like to make the css prefs write to the prefs db and have updating the theme be a side effect of db changes, like how blackbox does it

Screenshot from 2023-05-29 15-12-55

More Screenshots

Screenshot from 2023-05-29 15-13-14
Screenshot from 2023-05-29 15-13-25
Screenshot from 2023-05-29 15-13-41
Screenshot from 2023-05-29 15-13-47
Screenshot from 2023-05-29 15-13-51
Screenshot from 2023-05-29 15-13-54

@bennypowers bennypowers marked this pull request as ready for review May 29, 2023 16:17
@bennypowers
Copy link
Contributor Author

Ok this is ready for review. I'm sure there are lots of small things that I overlooked, and code style changes that you'd like to make.

I'd still like to refactor the colour preferences, but doing so without breaking backwards would be difficult, so that should wait for a further PR

panel.js Outdated Show resolved Hide resolved
preferences/about.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@jmmaranan jmmaranan left a comment

Choose a reason for hiding this comment

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

Hi @bennypowers,

Overall, it looks great and refreshing

  • My wish is that if we can bring back the state icons when the tiling mode is in workspace float, tiling off or on.
  • For the text, I would like your opinion on having the messages.js to contain the messages or we define any text directly on the place they are being used going forward. And then perhaps provide styleguide of some sort going forward
  • We can drop < 43. I assume just retaining 43, 44 in the metadata.json

Makefile Show resolved Hide resolved
extension.js Show resolved Hide resolved
indicator.js Outdated Show resolved Hide resolved
panel.js Outdated Show resolved Hide resolved
preferences/about.js Outdated Show resolved Hide resolved
preferences/keyboard.js Show resolved Hide resolved
settings.js Outdated Show resolved Hide resolved
@jmmaranan jmmaranan added enhancement New feature or request config-mgt Configuration management components labels May 29, 2023
@bennypowers
Copy link
Contributor Author

Thank you for the feedback. I will implement a "settings" window that combines experimental and developer

For the icon, what if we kept the "indicator only when on", but updated the quick settings icon depending on state?

@jmmaranan
Copy link
Collaborator

Thank you for the feedback. I will implement a "settings" window that combines experimental and developer

Sounds good. The developer section is tied to the production switch.

For the icon, what if we kept the "indicator only when on", but updated the quick settings icon depending on state?

  • The Panel icon (along with wifi, etc), would have the same behavior as it is today (and one small change on the Workspace floating icon - more on below). Icon only gets removed when Forge is disabled or uninstalled from Ext Manager or similar.
  • When Tiling is Off or the current workspace is floating, Quick settings icon should be the icon you used on the quick settings - showing as two separate boxes / windows. For the Panel icon, use the same two boxes icon. The current production workspace floating icon is different but let's drop it. So that there are only two icons to keep it simpler.
  • When Tiling is On or the current workspace is Tiled, Quick settings icon should be the current production tiled icon. You already got the panel icon correct on this PR.

Summary:

  • Two columns icon (Tiling ON/Workspace Tiling ON)
  • Two boxes icon (Tiling Off / Workspace Tiling Off)
  • Panel icon / Quick settings only disappears when Forge is disabled or removed

@bennypowers
Copy link
Contributor Author

Ok I'll take a look, that might be contrary to the HIG though. The built-in class and tutorial says

You may also want your extension to show a panel indicator when the feature is enabled.

Compare that to how the night light or WiFi panel icons work. The intent of the quick settings panel is to remove information from the panel whenever features are disabled

@jmmaranan
Copy link
Collaborator

jmmaranan commented May 29, 2023

Ok I'll take a look, that might be contrary to the HIG though. The built-in class and tutorial says

You may also want your extension to show a panel indicator when the feature is enabled.

Compare that to how the night light or WiFi panel icons work. The intent of the quick settings panel is to remove information from the panel whenever features are disabled

If that's the case, let's follow the interface guidelines and please update the icon to view-grid-symbolic instead:

image

Again, thank you for your time and effort on this. 👍🏼 Just waiting for the Experimental and Developer sections and I think it should be good to be merged.

@bennypowers
Copy link
Contributor Author

bennypowers commented May 30, 2023

  • Merged Experimental and Developer into "Settings"
  • Make "Settings" first page
  • Rename "experimental" group to settings and add an "experimental" flag to switch widget
  • Added an AdwAboutWindow accessible by clicking a header button in "Settings"
  • Derive contributor metadata from git log and present that in About
  • Remove panel button / drop support for GNOME < 43
  • Remove make dev and add make text-{x,wayland}

@bennypowers
Copy link
Contributor Author

I moved some preferences around and refactored in c56f5fe. most of the changes wre "best guesses"

@jmmaranan
Copy link
Collaborator

This looks really good to me. Thank you! I just gave the AUR packager a heads up. I think they were using make build - but I will go ahead and merge this.

@jmmaranan
Copy link
Collaborator

@bennypowers - looks like your focus hint changes was not rebased when you started this branch, do you know if you can fix that?

wip: migrate most settings pages to libadwaita

fix: about page

wip: prefs

feat: adwaita prefs

fix: keyboard settings

refactor: modularize prefs window

feat: quick settings

refactor: appearancepage

fix: quick settings displays above background apps

fix: nicer reset buttons in colour rows

fix: remove log

feat!: add about dialog

remove shell <43 support

fix: disable production

feat: remove 'experimental' group in favor of flagging settings

fix: makefile fixes

fix: move preferences around

and refactor preferences page
@bennypowers
Copy link
Contributor Author

bennypowers commented May 31, 2023

ok i see, I merged instead of rebasing. hopefully the gnarly rebase didn't break anything. I ran a quick test and it looked ok

@jmmaranan jmmaranan merged commit 380199f into forge-ext:main May 31, 2023
@bennypowers
Copy link
Contributor Author

Amazing! let's celebrate with a bowl of San Antonio Chili 🌶️ 🍖 🧅

@bennypowers bennypowers deleted the feat/prefs-adw branch May 31, 2023 11:58
@jmmaranan
Copy link
Collaborator

It's sent to extensions gnome dot org for review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config-mgt Configuration management components enhancement New feature or request gnome-43 gnome-44 GNOME 44
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: optimize/update preferences window feat:add a toggle to hide the topbar button or move it to QS
2 participants