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 a Project Search button to the Status Bar #19238

Conversation

HarshNarayanJha
Copy link
Contributor

@HarshNarayanJha HarshNarayanJha commented Oct 15, 2024

Closes #7836

Release Notes:

  • Added a little Project Search button to the status bar for anyone who wants quick access to it

image

Note: The ProjectSeachIndicator I created has a lot of potential as showing the project search stats for a global search, just like diagnostics items.rs (DiagnosticsIndicator) does.

EDIT: For Design issues, please let me know if search icon on the right side of status bar would be more pleasing

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Oct 15, 2024
@HarshNarayanJha HarshNarayanJha changed the title Add a Project Search button to the Status Bar. Add a Project Search button to the Status Bar Oct 15, 2024
@maxdeviant
Copy link
Member

cc @zed-industries/design

@HarshNarayanJha
Copy link
Contributor Author

Any issues with the PR?

@baldwindavid
Copy link
Contributor

If this is added we'll want to be able to hide for people that don't like seeing things they'll never use ✋. The convention for hiding these is a button: bool setting (e.g. collaboration_panel, chat_panel, notification_panel, assistant, terminal, outline_panel, project_panel).

@HarshNarayanJha
Copy link
Contributor Author

Okay I added the setting

"project_search": {
	"button": false
}

will hide it (hopefully, compiling right now)

So where else do I need to document this setting? (schemas, docs etc)

(Btw I just noticed that project search doesn't render file icons, maybe we can add those too!)

@mrnugget
Copy link
Member

Not against it and also not a designer, but throwing my hat in the ring by saying I think it should be hidden by default. (We do have a discoverability problem, but I think we should fix that in a way that's not adding buttons for features people don't know about.)

@HarshNarayanJha
Copy link
Contributor Author

should be hidden by default

Sure, that's just a matter of changing the default settings. Let me know if thats preferable

I have implemented the settings already, i will push by tonight

@danilo-leal
Copy link
Contributor

Sweet—thanks for the PR! I think it makes sense to expose this button and have it hidden by default. Also pushed a small commit to use the existing Icon Button instead of ButtonLike.

I do have one thought that I’d like to share, for the record, although I don’t yet have a clear solution/alternative: I feel like it’s a bit weird to have the search button in the status bar as it doesn’t behave the same way as the other buttons, all of which trigger panels.

Like, you click on it and open up the Project Search tab, but clicking on it back again doesn’t close it. It's not a toggle. And the icon button also never appears as activated (with the primary color, as the other buttons do). The diagnostics button sort of suffers from the same problem. This makes it feel like they’re not consistent with the remaining buttons from the status bar. If we ever expose a way for our Project Search to appear within a panel instead of a standalone tab, the button’s placement and behavior would feel more cohesive (which is also more similar to VS Code’s approach). I almost want to experiment with adding it somewhere else in the UI due to all of this, but I haven’t explored much yet. I just quickly experimented with adding a divider between the button "groups", just so to create some visual separation as they behave differently, but maybe it’s just noise.

image

TL;DR: These thoughts shouldn’t be a blocker for moving forward!

@baldwindavid
Copy link
Contributor

@danilo-leal I share some of the same thoughts. Also, for posterity, I'd note that there is an existing way to get at project search via the tab bar quick actions (which is another button group I'd like to be able to hide)...

2024-10-16T122538@2x

@HarshNarayanJha
Copy link
Contributor Author

Thanks for the commits. I haven't fully discovered the gpui's apis yet, and my poor laptop can't handle rust intellisense for this big codebase 😓. I just tried to match existing implementations for the project panel button and diagnostics button and built upon that.

I do have one thought that I’d like to share, for the record, although I don’t yet have a clear solution/alternative: I feel like it’s a bit weird to have the search button in the status bar as it doesn’t behave the same way as the other buttons, all of which trigger panels.

Like, you click on it and open up the Project Search tab, but clicking on it back again doesn’t close it. It's not a toggle. And

I completely agree with you, but again I just took the existing implementation for the diagnostics button and built upon that. The divider you added is honestly the best approach from my viewpoint. It does separate the left panels and the tabs 👍🏼. If agreed by others, you may push that divider too. I would rather add a similar divider to the right side (separating cursor info, bottom panels and right panels). And the current padding does that already, if you carefully notice.

@HarshNarayanJha
Copy link
Contributor Author

@danilo-leal I share some of the same thoughts. Also, for posterity, I'd note that there is an existing way to get at project search via the tab bar quick actions (which is another button group I'd like to be able to hide)...

Yeah, I am aware of that option, and my initial plan was to add the search icon to the top bar, but then the bottom status bar seemed to be a better place for the diagnostics button, which is kind of similar.

@baldwindavid
Copy link
Contributor

If we ever expose a way for our Project Search to appear within a panel instead of a standalone tab, the button’s placement and behavior would feel more cohesive (which is also more similar to VS Code’s approach).

Might be nice though I tend to think some of this becomes moot if a telescope/helix-style live file and grep search/previewer is eventually introduced.

@HarshNarayanJha
Copy link
Contributor Author

Also pushed a small commit to use the existing Icon Button instead of ButtonLike.

Actually, this is causing me a small issue with implementing the hide the button setting. You are returning an IconButton, which has to have and visual representation, but I must not render it depending on the setting, which forces me to use a h_flex with and child of IconButton, which maybe None if setting instructs.

Also, I am removing the file_icons setting since there is no support for file icons in the project search tab

@HarshNarayanJha
Copy link
Contributor Author

Okay, so the setting works now and defaults to false. 🎉

The only problem is that after hiding the search button, the diagnostics button is offset by a small amount, even if I use

div().w(rems(0.0))

How to add an empty div right there?

image

@RemcoSmitsDev
Copy link
Contributor

RemcoSmitsDev commented Oct 16, 2024

I think you can use the Empty element:

pub struct Empty;

Empty.into_any()

@HarshNarayanJha
Copy link
Contributor Author

HarshNarayanJha commented Oct 16, 2024

Didn't work with Empty, still having the spacing. Even tried to zero margin, padding and width of a div, but still has the space reserved.

Apart from that, I think is PR is ready for merging

@RemcoSmitsDev
Copy link
Contributor

Yeah, I think GPUI doesn't take this into account yet.

@HarshNarayanJha
Copy link
Contributor Author

For the space that is wasted by this button, moving the search button next to the diagonistics button helps since the diagonistics text doesn't always stays visible. So space is not wasted

@maxdeviant
Copy link
Member

Thank you for your work on this PR. After some internal discussion we've decided we don't want to move forward with adding this button.

Given it's not something we want enabled by default, having it as a setting just means that it's something extra that we need to take into account when designing in this area.

We're trying to keep the status bar as lightweight as possible.

@maxdeviant maxdeviant closed this Oct 21, 2024
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.

Add "Find In Project" button and keyboard shortcut
6 participants