-
Notifications
You must be signed in to change notification settings - Fork 5.9k
Fix spacing around hidden status bar items #39992
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 spacing around hidden status bar items #39992
Conversation
249f06e to
600c416
Compare
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.
Thanks for the PR! I left a little comment, but I think it'd be probably better if you could break the changes into separate pull requests. For example, adding the line ending indicator setting can be a follow-up. It'd be great to make this one focused just on the spacing improvements. Ping me once you've extracted out that bit out of this PR.
crates/gpui/src/styled.rs
Outdated
|
|
||
| /// Sets the display type of the element to `none`. | ||
| /// [Docs](https://tailwindcss.com/docs/display) | ||
| fn none(mut self) -> Self { |
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.
| fn none(mut self) -> Self { | |
| fn hidden(mut self) -> Self { |
I think this is a bit clearer and more consistent with Tailwind.
600c416 to
94c8c98
Compare
|
Sure thing! @danilo-leal I did notice a problem, children of I tried fixing it at least for this case: div().hidden().child("oh no");But I'm not sure if those are the only places child elements need to be skipped over.. |
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.
Nice, this looks good; thank you for following up!
This is a follow-up PR to zed-industries#39609, and attempts to address hidden status bar items still contributing to the layout and creating extra spacing.  - 203cbd6 Adds a `.none()` method to the `gpui::Styled` helper trait, so that status items can set their display type to none inside their `render` method. - 249f06e Applies `.none()` to all the status items. - ~~499f564906c88336608c81615b11ebc9ab43d832~~ At first I was adding an `is_visible` method to the `StatusBarView` trait, which would be used to skip status bar items which would just render an empty div anyway, but I felt duplicating the conditions for hiding the buttons between the status items `is_visible` and `render` methods could be an attraction for bugs, so I tried to find another approach. This commit contains those changes, reverted immediately (if the `is_visible` approach is preferred I can bring it back!) - f37cb75 (bonus!) Adds a condition to the vim mode indicator to avoid a leading space when there are no pending keys. Release Notes: - N/A
Closes #5294 This PR adds a line ending indicator to the status bar, hidden by default as discussed in #5294. ### Changes - 8b063a2 add the indicator and `status_bar.line_endings_button` setting. - ~~9926237b709dd4e25ce58d558fd385d63b405f3b changes `status_bar.line_endings_button` from a boolean to an enum:~~ <details> <summary> show details </summary> - `always` Always show line endings indicator. - `non_native` Indicate when line endings do not match the current platform. - `lf_only` Indicate when using unix-style (LF) line endings only. - `crlf_only` Indicate when using windows-style (CRLF) line endings only. - `never` Do not show line endings indicator. I know this many options might be overdoing it, but I was torn between the pleasant default of `non_native` and the simplicity of `lf_only` / `crlf_only`. My thinking was if one is developing on a project which exclusively uses one line-ending style or the other, it would be nice to be able to configure no-indicator-in-the-happy-case behavior regardless of the platform zed is running on. But I'm not really familiar with any projects that use exclusively CRLF line endings in practice. Is this a scenario worth supporting or just something I dreamed up? </details> - 0117419 rename the action context for `line ending: Toggle` -> `line ending selector: Toggle`. When running the action in the command palette with the old name I felt surprised to be greeted with an additional menu, with the new name it feels more predictable (plus now it matches `language_selector::Toggle`!) ### Future work Hidden status bar items still get padding, creating inconsistent spacing (and it kind of stands out where I placed the line-endings button): <img alt="the gap after the indicator is larger than for other buttons" src="https://github.com/user-attachments/assets/24a346d4-3ff6-4f7f-bd87-64d453c2441a" /> I started a new follow-up PR to address that: #39992 Release Notes: - Added line ending indicator to the status bar (disabled by default; enabled by setting `status_bar.line_endings_button` to `true`)
This is a follow-up PR to #39609, and attempts to address hidden status bar items still contributing to the layout and creating extra spacing.
203cbd6 Adds a
.none()method to thegpui::Styledhelper trait, so that status items can set their display type to none inside theirrendermethod.249f06e Applies
.none()to all the status items.499f564At first I was adding anis_visiblemethod to theStatusBarViewtrait, which would be used to skip status bar items which would just render an empty div anyway, but I felt duplicating the conditions for hiding the buttons between the status itemsis_visibleandrendermethods could be an attraction for bugs, so I tried to find another approach. This commit contains those changes, reverted immediately (if theis_visibleapproach is preferred I can bring it back!)f37cb75 (bonus!) Adds a condition to the vim mode indicator to avoid a leading space when there are no pending keys.
Release Notes: