search: Fix collapse/expand all button sync#48773
search: Fix collapse/expand all button sync#48773dinocosta merged 8 commits intozed-industries:mainfrom
Conversation
Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com>
dinocosta
left a comment
There was a problem hiding this comment.
Thank you for tackling this @bnjjj ! 🙂
Changes are looking good but I suspect that, if we're updating this, we can probably also remove BufferSearchBar.is_collapsed and ProjectSearchView.results_collapsed . I also noticed that we still have a bug in case the button is used when focused on the search query editor, as that ends up calling the BufferSearchBar.toggle_fold_all_in_item method which still relies on BufferSearchBar.is_collapsed ▼
CleanShot.2026-02-10.at.15.35.57.mp4
Notice how, even though the button says, "Expand All Files", as it was updated, after collapsing a single file, clicking it actually collapses all files .
Let me know if you have any questions. Thanks!
Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com>
|
Thanks for the review @dinocosta I pushed a new commit which fix the project search bar. I tested manually and it worked properly. Let me know if you're able to reproduce your bug. |
There was a problem hiding this comment.
@bnjjj Thank you for tackling that change to BufferSearchBar.toggle_fold_all_in_item ✌️
Since we're now relying on the Editor.has_any_buffer_folded method as the source of truth for which icon and tooltip to show, I believe we can still remove both the BufferSearchBar.is_collapsed and ProjectSearchView.results_collapsed fields .
In the case of the Render implementation for BufferSearchBar, I believe we can either:
- Opt for a simpler approach and just use
.unwrap_or(false)where we're currently doing.unwrap_or(self.is_collapsed) - Update the
SearchableItemHandletrait with ahas_results_collapsedmethod and avoid downcasting
Let me know what you think. Thanks! 🙂
|
I also agreed with you @dinocosta but I discovered that line https://github.com/bnjjj/zed/blob/bnjjj/fix_9880/crates/search/src/buffer_search.rs#L1387 which requires to have a kind of local state. Do you have any clue on how to handle that ? Should I update the folded buffers in editor.rs ? |
@bnjjj I believe we can get rid of the assignment to |
Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com>
|
thanks @dinocosta ! I think I addressed your comments |
Since we're now simpy relying on the `Editor.has_any_buffer_folded` method in order to understand whether any buffer is folded, in order to display the correct icon and tooltip, we no longer need to track this state in a different field, so this commit removes it, just like it was done for `BufferSearchBar.is_collapsed`. It also includes a fix to a test that was providing a `WeakEntity<MultiWorkspace>` to `ProjectSearchView::new`, which accidentally got in when merging `main` to this branch.
* Remove `workspace::searcahable::CollapseDirection` as we no longer need to know whether the reuslts have been collapsed or expanded, as we're now simply relying on the `Editor.has_any_buffer_folded` method. * Remove `workspace::searcahable::Search::ResultsCollapsedChanged` event as it was emitted on both `Editor.fold_all` and `Editor.unfold_all` methods. However, since these methods end up calling `Editor.fold_buffer` and `Editor.unfold_buffer`, respectively, a `cx.notify` call is already being triggered, and since both project search and buffer search area already observing the results editor, they're re-rendered when a single buffer is folded/unfolded, so we no longer need a specific event for when all results are collapsed or expanded.
dinocosta
left a comment
There was a problem hiding this comment.
@bnjjj Thank you for tackling the comments! I've also removed the ProjectSearchView.results_collapsed field and the SearchEvent::ResultsCollapsedChanged event, as it's no longer needed for the UI to re-render with the correct icon and tooltip 🙂
Field was accidentally leftover when merging `main` to this branch, as these changes have been reverted.
Derive collapsed state from `Editor.has_any_buffer_folded` instead of tracking it separately, removing redundant `ResultsCollapsedChanged` event and stale `is_collapsed`/`results_collapsed` fields. Closes #48734 Release Notes: - Fixed collapse/expand all button in buffer search and project search not syncing correctly when toggling individual file sections --------- Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com> Co-authored-by: dino <dinojoaocosta@gmail.com>
Closes #48734
Release Notes: