-
Notifications
You must be signed in to change notification settings - Fork 18
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
Added new color for visible (but unfocused) workspace and its config value #50
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have two really minor nitpicking comments about whitespace.
The name "visible" is fine, I have no better idea.
add_color_picker(config, dialog_vbox, "Focused Workspace Color:", config->focused_color, focused_color_changed); | ||
add_color_picker(config, dialog_vbox, "Urgent Workspace Color:", config->urgent_color, urgent_color_changed); | ||
add_color_picker(config, dialog_vbox, "Unfocused Visible Workspace Color:", config->visible_color, visible_color_changed); | ||
add_color_picker(config, dialog_vbox, "Binding Mode Color:", config->mode_color, mode_color_changed); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indentation is off
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand. Are the add_color_picker
's indented 4 more spaces on purpose? I put them on the same level as the surrounding elements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wait, you are right. The previous indentation was wrong. Sorry about that.
panel-plugin/i3w-config.c
Outdated
GdkColor color; | ||
gtk_color_button_get_color(GTK_COLOR_BUTTON(button), &color); | ||
config->visible_color = serialize_gdkcolor(&color); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indentation is off
Cheers! |
As we mentioned in #49, I implemented the new color in the configuration.
About the naming, inside the code I have chosen to call this new color "visible", since that is the naming used by i3ipc-glib. At the config UI I have called it "Unfocused Visible Workspace Color", but feel free to suggest any other name that suits this better.
Also, I thought that the ternary comparison to decide the color would get a bit too big now that there are 4 different colors for workspace labels, so I replaced that by an if-else chain which is more readable IMO.
The rest of the changes are pretty self-explanatory