Moon-0xff / gnome-mpris-label

A music related GNOME extension.
GNU General Public License v3.0
50 stars 9 forks source link

option to specify label font color #94

Closed Batwam closed 4 months ago

Batwam commented 5 months ago

implement solution for issue #93 to specify label font color

image

image

Batwam commented 5 months ago

updated to include a color choose widget (and use rgb value in the background): image

Moon-0xff commented 5 months ago

I think this option shouldn't have a default value (or have a null value as default), and the code should be executed only if the user has picked something.
Font colors are dependent on the shell theme, and our default might blend with the background.

PS: Check your email!

Batwam commented 5 months ago

I agree with the fact that assuming that white is the default can be dangerous if people have themes. The main challenge I had is that, while it's probably possible to figure it out, I don't know how to find out what that default colour is. This said, I could leave the default setting blank. This will have the effect to set_style('color:""') which seems to set the colour back to the default without throwing any errors.

The Setting page then looks like this with a transparent colour (since the colour isn't recognised) which indicates that the default font isn't being overwritten): image

Is that good/clear enough? If not, we'd need to find a way to extract the default colour so it can be applied to the ColorButton when the setting is blank.

Batwam commented 5 months ago

ok, I found how to get the font colour using this post. As far as I can tell, it's not possible to import Main in prefs.js (which was also the issue the person posting had) so I followed the same solution and created a separate setting which is used to store this default value. This setting is being populated/refreshed on startup and whenever the color is getting changed.

image

It works, but I really don't like having to create this extra setting key... The sole reason I'm doing this is so I can set the colorPicker's colour correctly in the preferences window to match the theme's font when the font color is being reset. Otherwise, the previous solution with the checkered rectangle works as well doesn't necessitate the additional code.

Batwam commented 5 months ago

I have reverted the previous commit. it was possible to find the font colour but this added too much complexity to the code and potential hedge cases when trying to keep addColorPicker() generic for future uses, depending on whether an alternative key exists or (which needs to be checked with possible fallback, need to make sure the key exists or it will crash,...), effect of the theme changing after the key has been initialised, ...

There is probably a way to do it using a separate utils.js and importing it but I don't have the time/patience and having to manage both gnome 42/43 and gnome45+ syntax with the patch makes this overly time consuming.

Moon-0xff commented 5 months ago

The behaviour explained here was good enough for me.

Displaying the correct default color is neat. But I agree (of course!) that it doesn't justify the complexity.

What's the default can be explained on the subtext (or tooltip).