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

Name root/parent module GTK objects. #57

Merged
merged 2 commits into from Jul 14, 2021
Merged

Name root/parent module GTK objects. #57

merged 2 commits into from Jul 14, 2021

Conversation

ghost
Copy link

@ghost ghost commented Jul 12, 2021

This allows the modules root object (the module object directly inheriting a GTK object) to be referenced in CSS (as 'root-' and whatever CSS name was configured for that module).

@ghost ghost mentioned this pull request Jul 12, 2021
Copy link
Owner

@nwg-piotr nwg-piotr left a comment

Choose a reason for hiding this comment

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

  1. nwg_panel/modules/controls.py

The Controls class derives from Gtk.EventBox. The line

check_key(settings, "css-name", "controls-label")

seems to have remained there by oblivion. I'm not sure if naming Gtk.EventBox "controls-label" makes sense.

  1. Nobody's going to learn about the new names, unless they appear in config GUI. Would you mind adding aproppriate fields in glade files, and their support in config.py?

@ghost
Copy link
Author

ghost commented Jul 13, 2021

1 . 'controls-label' was just the name already set in 'css-name' (granted it's subsequently overwritten in the Popup part, aside from never actually being set). Following point 2 I suppose I can look to add a second 'css-name' ('css-overview' ?) for this particular module (seeing as the Controls module actually has two main GTK parts, the icons overview and the Popup itself).

  1. Sure I'll have a look into that section and the Glade bit.

@nwg-piotr
Copy link
Owner

All right then. I tried to work on three various programs tonight, which won't result in anything good for sure. I fixed a bug in nwg-drawer, reviewed your PR, while still thinking about adding a feature to nwg-wrapper. I'll focus on the latter for now.

@ghost
Copy link
Author

ghost commented Jul 13, 2021

A 'Root CSS' settings value has been added to refer to the base/root object in modules 'executor', 'controls' and 'clock' (which all currently set the base CSS name to a different element). Adding this as a new setting preserved backwards compatibility (i.e. no ones style is going to be changed unless they opt-in to setting and using these selectors)

@ghost ghost requested a review from nwg-piotr July 13, 2021 23:40
Copy link
Owner

@nwg-piotr nwg-piotr left a comment

Choose a reason for hiding this comment

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

Seems to be OK. What I don't like is that the configuration gradually gets more and more difficult to ordinary users. But it's not your fault, of course.

@nwg-piotr nwg-piotr merged commit 886357a into nwg-piotr:master Jul 14, 2021
@nwg-piotr
Copy link
Owner

Haven't noticed at first: CSS names for executors disappeared, and I needed to add them again. Alright, let it stay as it is.

@ghost
Copy link
Author

ghost commented Jul 15, 2021

Seems to be OK. What I don't like is that the configuration gradually gets more and more difficult to ordinary users. But it's not your fault, of course.

Not sure what could be done to make that easier. Maybe a GUI re-jig (less explicit fields, maybe drop a number of them into separate combo-boxes? i.e. Add Action: Scroll Up, Left Click, etc.), however that would add a decent amount of complexity. Could also go the GNOME route and drop some options from the GUI but document them to be configurable in the configuration file (i.e.Hide the options).

Haven't noticed at first: CSS names for executors disappeared, and I needed to add them again. Alright, let it stay as it is.

Disappeared? I assume you mean they weren't read from the configuration file into the GUI? That's strange, I didn't encounter this, and I'm not sure what would cause this... Best as I can tell the changes were all additive.

Thanks for the merge. I don't think I have anything else to change for my workflow (I just needed this change so I can have nice hover shadows around the modules I'm selecting).

@nwg-piotr
Copy link
Owner

nwg-piotr commented Jul 15, 2021

Well, it's always better than to edit json.

Could the release wait a few days? I'd like to integrate a plugin, but I need a weekend and some beer to deal with it.

@ghost
Copy link
Author

ghost commented Jul 15, 2021

By all means (I'm just building from master anyway).

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

1 participant