Fix search / find panel - #4236
Conversation
|
Hi @SableRaf! I took a pass at the issue we discussed and ended up having to do the custom implementation anyways so I was able to tackle more of your requests. want to take a look and let me know what you think? |
|
It looks amazing! One minor nitpick: in the mobile layout, having the close button right next to the expand/collapse button doesn’t feel quite right. It also seems like it could lead to accidentally closing the search panel. |
I think I fixed it! good idea mobile-cm-search-find.mov |
|
Hiii @raclim @clairep94 sorry to bug you guys! If you have time would you be able to look at this? Just want to make sure it doesn't fall through the cracks : ) |
| * Custom implementation of CodeMirror 6's built-in SearchPanel. | ||
| * | ||
| * TODO: | ||
| * We may need to add an announce functionality for screen readers, similar to the original: |
There was a problem hiding this comment.
I think this could be a great issue for folks!
raclim
left a comment
There was a problem hiding this comment.
Sorry for missing this, I thought I took a look and realized I never submitted 😅
I think this overall looks great to me!!! I think these improvements really align with the current editor appearance and adds a lot more clarity!
This is super minor, but I'm wondering if it might be worth having a fixed width for the search when a user toggles the dropdown chevron? I noticed the panel shifts in width between the two states (screenshots below), and wonder if keeping it consistent could reduce some visual movement. I'm not sure if this would might make the design a bit awkward, so definitely not a strong preference!
oooo good point! I did think about that, but if you're cool with it, i'd like to keep the current version because it keeps the zero state a little more condensed in design so that it doesn't overlap too much of the first line of code. wdyt? also great point about the accessibility TODO! I''ll open an issue after this merges in : ) |
|
also sorry to bother you again @raclim but i think i need another review since the code changed when i resolved some merge conflicts? |
|
Keeping the current design sounds good to me! Also no worries at all, I'll re-approve! |


Issue:
Fixes #4190
Demo:
desktop-cm-search-find.mov
mobile-cm-search-find.mov
Changes:
Creates a custom implementation of the search panel that more closely mirrors the vscode UX / UI design.
I have verified that this pull request:
npm run lint)npm run test)npm run typecheck)developbranch.Fixes #123