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

Add module class to the root elements of the modules #2921

Merged
merged 2 commits into from
Feb 16, 2024

Conversation

alebastr
Copy link
Contributor

@alebastr alebastr commented Feb 15, 2024

At some point, my draft style for Fedora configuration was looking like that:

Warning: ugly CSS fragment inside
/* Common module rules */
.modules-left > widget > label,
.modules-center > widget > label,
.modules-right > widget > label {
  color: inherit;
  margin: 0;
  padding: 0 10px;
}

box.vertical.modules-left > widget > label,
box.vertical.modules-center > widget > label,
box.vertical.modules-right > widget > label {
  padding: 10px 0;
}

window#waybar:not(.bottom):not(.left):not(.right) .modules-left > widget > label,
window#waybar:not(.bottom):not(.left):not(.right) .modules-center > widget > label,
window#waybar:not(.bottom):not(.left):not(.right) .modules-right > widget > label {
  box-shadow: inset 0 -2px;
}

window#waybar.bottom .modules-left > widget > label,
window#waybar.bottom .modules-center > widget > label,
window#waybar.bottom .modules-right > widget > label {
  box-shadow: inset 0 2px;
}

window#waybar.left .modules-left > widget > label,
window#waybar.left .modules-center > widget > label,
window#waybar.left .modules-right > widget > label {
  box-shadow: inset -2px 0;
}

window#waybar.right .modules-left > widget > label,
window#waybar.right .modules-center > widget > label,
window#waybar.right .modules-right > widget > label {
  box-shadow: inset 2px 0;
}

(Actually, it was worse - there's also a .modules-xxx > widget > box. And #image)

Two obvious ideas to optimize that:

  • Ensure that the position class on windows is always set and :not(.bottom):not(.left):not(.right) can be shortened to .top.
  • Introduce a shorter selector for each module, like label.module/box.module/etc...

After that, the style was reduced to a rather tolerable

/* Common module rules */
label.module { /* or .module:not(box) */
  color: inherit;
  margin: 0;
  padding: 0 10px;
}

box.vertical label.module {
  padding: 10px 0;
}

window#waybar.top label.module {
  box-shadow: inset 0 -2px;
}

window#waybar.bottom label.module {
  box-shadow: inset 0 2px;
}

window#waybar.left label.module {
  box-shadow: inset -2px 0;
}

window#waybar.right label.module {
  box-shadow: inset 2px 0;
}

Part of bar.cpp refactoring should also simplify adding support for position property of Sway IPC (see sway-bar(5)/get_bar_config in sway-ipc(7)). But that functionality does not belong here and is not finished yet.


A Styling wiki update could look like that (after Module Group Styling):

Generic module style

A style with the .module selector would affect all the modules. In practice, you may prefer to use more specific label.module and box.module selectors.

label.module {
    padding: 0 10px;
    box-shadow: inset 0 -3px;
}
box.module button:hover {
    box-shadow: inset 0 -3px #ffffff;
}

You need to remember about selector specificity when overriding the style for a specific module:

/*
 * Show border for all simple text modules when the bar is in a top or bottom position.
 * a=1 b=2 c=2 -> specificity = 122
 */
window#waybar.top label.module {
    box-shadow: inset 0 -3px;
}
window#waybar.bottom label.module {
    box-shadow: inset 0 3px;
}
/*
 * But hide the border for sway/window (need to include `window#waybar` to increase specificity)
 * a=2 b=0 c=1 -> specificity = 201
 */
window#waybar #window {
    box-shadow: none;
}

Ensure that the position and the corresponding CSS class on window are
always set.
Previously, the only way to select all the module labels was with the
following kind of selector:
```css
.modules-left > widget > label,
.modules-center > widget > label,
.modules-right > widget > label {
    /* ... */
}
```
(and a matching block for the `box` containers).

Now, this can be expressed as
```css
label.module, box.module {
    /* ... */
}
```
@Alexays
Copy link
Owner

Alexays commented Feb 16, 2024

LGTM thx :)

@Alexays Alexays merged commit 3cd3118 into Alexays:master Feb 16, 2024
7 of 9 checks passed
@alebastr alebastr deleted the module-classes branch February 16, 2024 18:32
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.

2 participants