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

feat: Smart Borders and Window effects for if a workspace only has one tiling child #933

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

Miitto
Copy link
Contributor

@Miitto Miitto commented Jan 21, 2025

Added a new config option under gaps called single_screen_gap (probably want a different name) which sets the outer gap sizes for any workspace with only one tiling window in it.

Feature requested in #886

Also fixed a possible bug where reading the config would fail if the home directory could not be found, even if a custom config path was passed that pointed elsewhere.

@lars-berger
Copy link
Member

Ay thanks for the PR’s ❤️ Rushing a bit currently to get a new release out with the latest batch of fixes + features. I can do some proper testing and review these after - shouldn’t be more than a couple of days

@Miitto Miitto changed the title feat: Add different gap sizes for if a workspace has only one window feat: Smart Borders and Window effects for if a workspace only has one tiling child Jan 25, 2025
@Miitto
Copy link
Contributor Author

Miitto commented Jan 25, 2025

Annoying force push there, it was that or a merge commit. Got some commits out of order with a rebase earlier I think.
Can't find anything overwritten, and all the commits made are there.

Any tips on how to avoid in future? I think something to do with cherry picking might help.


/// Gap between window and the screen edge if there is only one window
/// in the workspace
pub smart_outer_gap: Option<RectDelta>,
Copy link
Member

Choose a reason for hiding this comment

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

maybe single_window_outer_gap instead?

impl GapsConfig {
#[must_use]
#[allow(clippy::missing_panics_doc)]
pub fn get_outer_gap(&self, single_window: bool) -> &RectDelta {
Copy link
Member

Choose a reason for hiding this comment

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

we generally keep config getters within the impl UserConfig in user_config.rs. so for example:

  pub fn outer_gaps_for_workspace(
    &self,
    workspace: &Workspace,
  ) -> &RectDelta {
    let is_single_window = workspace.tiling_children().next().is_none();

    match &self.value.gaps.single_window_outer_gap {
      Some(outer_gap) if is_single_window => outer_gap,
      _ => &self.value.gaps.outer_gap,
    }
  }

  pub fn has_outer_gaps(&self, workspace: &Workspace) -> bool {
    let outer_gap = self.outer_gaps_for_workspace(workspace);

    // Allow for 1px/1% of leeway.
    outer_gap.bottom.amount > 1.0
      || outer_gap.left.amount > 1.0
      || outer_gap.right.amount > 1.0
      || outer_gap.top.amount > 1.0
  }

@@ -220,20 +235,47 @@ pub struct WindowEffectsConfig {
#[serde(default, rename_all(serialize = "camelCase"))]
pub struct WindowEffectConfig {
/// Config for optionally applying a colored border.
pub border: BorderEffectConfig,
pub border: SmartBorderEffectConfig,
Copy link
Member

Choose a reason for hiding this comment

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

i'm fully on board with the gaps feature, but there's a few things to figure out re. the smart borders. is it mainly for the sake of if someone is running a single window outer gap of 0px? trying to get a better understanding of the use case.

if the purpose is primarily to remove border/corner effects if the window takes up the full monitor, maybe it'd make more sense to have an option for disabling window effects if a window is fullscreen or is a single window with an outer gap of 0px? either way, imo the best would be to figure out the exact use case and then potentially pr a solution for that separately

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 📬 Needs triage
Development

Successfully merging this pull request may close these issues.

2 participants