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

Allow configuring spacing of project panel entries #16255

Merged
merged 6 commits into from
Jan 9, 2025

Conversation

mikesun
Copy link
Contributor

@mikesun mikesun commented Aug 14, 2024

Release Notes:

  • Added project_panel.entry_spacing setting to configure spacing between entries in the project panel.

Comfortable (default)

  "project_panel": {
    "entry_spacing": "comfortable",
Screenshot 2024-08-14 at 5 50 41 PM

Standard

  "project_panel": {
    "entry_spacing": "standard",
Screenshot 2024-08-14 at 5 50 54 PM

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Aug 14, 2024
@maxdeviant maxdeviant changed the title Configuration of project panel worktree entries spacing Allow configuring spacing of project panel worktree entries Aug 14, 2024
@maxdeviant maxdeviant self-assigned this Aug 20, 2024
@mikesun
Copy link
Contributor Author

mikesun commented Sep 15, 2024

Any feedback/comments on this approach?

@notpeter
Copy link
Member

notpeter commented Sep 16, 2024

Apologies on what may feel like bikeshedding, but I think if we're going to expose this as a setting, we should attempt to make it match terminal_line_height and buffer_line_height as much as possible:

- Description: The default line height for text in the editor.
- Setting: `buffer_line_height`
- Default: `"comfortable"`
**Options**
`"standard"`, `"comfortable"` or `{"custom": float}` (`1` is very compact, `2` very loose)

So:

  1. Instead of entry_spacing maybe entry_height?
  2. Instead of ['comfortable', 'compact'] maybe ['comfortable', 'standard']
  3. Maybe support custom spacing too: e.g. {"custom": 1.25}

Any feedback/comments on this approach?

CC design: @iamnbutler / @danilo-leal

@danilo-leal
Copy link
Contributor

danilo-leal commented Sep 16, 2024

Agree with matching the naming pattern we've been using for settings similar to this. I'm just not sure about "entry" as the word for it; feels like not be best one. I'd probably use item. So:

  "project_panel": {
    "item_spacing": "comfortable",

@mikesun
Copy link
Contributor Author

mikesun commented Nov 24, 2024

@notpeter danilo-leal Sorry for the delayed response.

Apologies on what may feel like bikeshedding, but I think if we're going to expose this as a setting, we should attempt to make it match terminal_line_height and buffer_line_height as much as possible:

- Description: The default line height for text in the editor.
- Setting: `buffer_line_height`
- Default: `"comfortable"`
**Options**
`"standard"`, `"comfortable"` or `{"custom": float}` (`1` is very compact, `2` very loose)

So:

  1. Instead of entry_spacing maybe entry_height?
  2. Instead of ['comfortable', 'compact'] maybe ['comfortable', 'standard']
  3. Maybe support custom spacing too: e.g. {"custom": 1.25}

So the issue is this isn't actually a height setting similar terminal_line_height and buffer_line_height where you can actually give a spacing value because the project panel items are rendered as list items. It's a bit of a hack that uses a negative pixel offset for the list item component:

ListItemSpacing::ExtraDense => this.py_neg_px(),

But naming-wise, I could rename it so?

  1. Instead of entry_spacing, item_height
  2. Instead of ['comfortable', 'compact'] maybe ['comfortable', 'standard']

Does that work? Happy to make those changes

@maxdeviant
Copy link
Member

Agree with matching the naming pattern we've been using for settings similar to this. I'm just not sure about "entry" as the word for it; feels like not be best one. I'd probably use item. So:

  "project_panel": {
    "item_spacing": "comfortable",

I think we should keep it "entry", as that's the terminology we use elsewhere (both in the code and in our actions and such).

@maxdeviant maxdeviant changed the title Allow configuring spacing of project panel worktree entries Allow configuring spacing of project panel entries Jan 9, 2025
Copy link
Member

@maxdeviant maxdeviant left a comment

Choose a reason for hiding this comment

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

Thank you!

@maxdeviant maxdeviant enabled auto-merge January 9, 2025 17:45
@maxdeviant maxdeviant added this pull request to the merge queue Jan 9, 2025
Merged via the queue into zed-industries:main with commit 9ea7ed8 Jan 9, 2025
14 checks passed
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