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

Let the project panel respond to ui_density settings #20460

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Remco-Schoeman
Copy link

@Remco-Schoeman Remco-Schoeman commented Nov 9, 2024

@iamnbutler edit: Density tracking issue:

I noticed a setting in the Zed config: unstable.ui_density while looking for ways to get more text on my cheap 1920x1080 second monitor. (I already had made the font 14px instead of the default 16px.)

While changing that setting to compact it did exactly what I wanted, I noticed the Project panel didn't play along. The panel didn't shrink the vertical whitespace between items which made it look a bit too 'airy'.

zed-before-density

So, I fixed it.

zed-after-density

The diff seems large, but it's mostly whitespace because of the indentation changes.

The actual change is pretty small:

zed-changeset

I've tried to see if and how I could add tests for this change,
but I couldn't find an existing example for a test for a visual change like this.
Setting up such a test myself is beyond me as Rust is mostly arcane magic to me, so any assistance from somebody who does know is welcome.

Release Notes:

  • Improve unstable.ui_density setting to apply to project panel

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Nov 9, 2024
@Remco-Schoeman Remco-Schoeman changed the title I've made the project panel respond to ui densitiy settings I've made the project panel respond to ui density settings Nov 9, 2024
@Remco-Schoeman Remco-Schoeman changed the title I've made the project panel respond to ui density settings Let the project panel respond to ui_density settings Nov 9, 2024
@zed-industries-bot
Copy link

zed-industries-bot commented Nov 10, 2024

Messages
📖

This PR includes links to the following GitHub Issues: #18078
If this PR aims to close an issue, please include a Closes #ISSUE line at the top of the PR body.

Generated by 🚫 dangerJS against 77cab80

@iamnbutler
Copy link
Member

iamnbutler commented Nov 10, 2024

Hey Remco! Thanks for working on expanding support for UI Density.

There is actually an even easier way to do this:

Rather than even accessing the ui_density setting you can use ui::Spacing. It was built for this exact purpose, to allow defining spacing with density in mind.

So you could do something like this:

CleanShot 2024-11-10 at 09 12 22@2x

And it would automatically respond to density. However the scale I have set up in Spacing doesn't go high enough to account for this case.

If you can give me a day or so to update the spacing scale in a separate PR, I'd prefer for us to use that approach as it keeps the way we build UI to respond to density consistent.

Alternatively, if you want to land this soon, you can use Spacing::custom_spacing for now and I can update it after I update the scale.

h_flex().h_6().w_full().child(editor.clone())
h_flex()
.map(|this| match ui_density {
theme::UiDensity::Comfortable => this.h_6(),
Copy link
Member

Choose a reason for hiding this comment

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

Let's use ui::Spacing rather than accessing the settings here!

@iamnbutler
Copy link
Member

iamnbutler commented Nov 11, 2024

Following up, I have a WIP PR up here: #20504. I'll add relevant details there. This should cover the needs of this PR however!

I will land this PR today.

…ject-panel-ui-density

# Conflicts:
#	crates/project_panel/src/project_panel.rs
@mikayla-maki
Copy link
Contributor

Hi @Remco-Schoeman , it seems whatever changes where in this PR got blown away by the merge. I'm going to set this to draft, feel free to mark it ready for review once this PR is back in shape :)

@mikayla-maki mikayla-maki marked this pull request as draft December 13, 2024 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants