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

Fix background/text color and typo #29

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

mmattamala
Copy link

Hi @heuristicus , this tries to fix minor things:

  • A minor typo in the clear command (it used to say _Clear)
  • The GTK methods used to change the background and font color in the Sheriff's command console, since the API changed and it was not working. While they work in Noetic, the override_background_color and override_color methods are also deprecated in favor of CSS-based formatting (I guess that's like Qt). However, I did a quick search I couldn't find a good example to implement it myself. Do you have any experience with this?

@mmattamala mmattamala requested a review from heuristicus July 12, 2021 14:38
@heuristicus
Copy link

This doesn't quite work, at least for me. The new GTK stuff takes the theme of the operating system and uses that to colour all components of the sheriff window. I'm currently using the Yaru-dark theme (according to gnome-tweak-tool), which I set by going to appearance>dark in the ubuntu settings. By default my sheriff looks like this:

Screenshot from 2021-07-13 09-32-51

However, when I use this branch, the console is set to a different colour than the rest of the interface:
Screenshot from 2021-07-13 09-35-12

This branch creates a configuration in ~/.config/procman_ros-sheriff/config which stores this change.

The _Clear fix is fine I think, since we don't really want to use mnemonics on right click actions. When you see an underscore in a GTK menu item, that usually indicates what the alt-key combination is to activate that item. For example, C_lear would mean that we can press alt+l to activate the clear menu item.

@mmattamala
Copy link
Author

Thanks for the feedback. I think that the second behavior is the one that procman used to have (because the console is white by default according to the code) but is inconvenient when using dark themes - that look way nicer to be fair.

The problem I faced is that when I tried to change the color of the terminal from the settings, the master branch wasn't allowing to do so, while I think it should be possible with this branch.

If there is a way to set the console color as the one from the operating system by default, but also allowing it to change it if required, it would be ideal I think. What do you think?

Regarding the Clear think, thanks for the clarification, I had no idea about that :)

@heuristicus
Copy link

I have an issue on the appearance thing at #23. I think it should be possible but it's relatively low priority given that it's cosmetic and not functional. It should be possible to do it but I'd have to dig through exactly how to do it since I'm not really familiar with GTK.

In terms of CSS formatting, so far I've only done it with GUI elements which attach to specific CSS which is predefined, see spot estop gui at https://github.com/ori-drs/spot_runtime_drs/blob/master/scripts/estop_gui.py#L47-L56. Maybe it's possible to set a CSS definition on each UI element but I haven't looked at that.

@mmattamala
Copy link
Author

Ok, no problem. I think I'll work locally with that fix since apart from that everything works fine with procman on my side. If I have some spare time I'll take another look and will update this PR.

@wxmerkt
Copy link
Member

wxmerkt commented Feb 1, 2023

Hi, any pointers or thoughts on possible fixes that would work and adapt to the theme (or just have a better contrast background by default?)? ROS_WARN messages are really hard to read on regular Yaru/Ubuntu-light default. Or perhaps my config is broken? Any advice appreciated :-)

image

@mauricefallon
Copy link

the color selector used to work (File--> Preferences). But it doesnt anymore
@heuristicus : do you know how to fix that?

image

That's the simplest solution.
another solution would be to capture all strings and change all to black. I dont know how to do that either.

@wxmerkt
Copy link
Member

wxmerkt commented Feb 4, 2023

Thank you very much for the pointers! I've been digging into this a bit, and here are some findings:

  1. Matias' PR fixes the File>Preferences color picker, but indeed does not work with picking up background colours from the theme.
  2. I have been able to get readable console output by changing yellow to yellow3 in the ANSI to color mapping. Since this fixes my primary issue I will make a separate PR with just these changes. Colour options are listed below
  3. I've checked with the Yaru-dark theme and it appears to work with Matias fix and the colour change per the preferences/color picker. The console stays white per File>Preferences settings, however, matching what Michal reported above.

These are the yellow colour options:

$ cat /etc/X11/rgb.txt | grep yellow
173 255  47		green yellow
154 205  50		yellow green
250 250 210		light goldenrod yellow
255 255 224		light yellow
255 255   0		yellow
255 255	  0		yellow1
238 238	  0		yellow2
205 205	  0		yellow3
139 139	  0		yellow4

Yaru-dark theme with yellow3 for readability: (incl. Matias' fix which keeps the console background per the user settings rather than theme/system settings)
image

Yaru-light theme with yellow3 for readability:
image

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.

4 participants