feat: add command menu for navigation #20
Conversation
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
|
Not sure whats up with the preview doesn't seem to have any of my changes |
|
Hello, @ieedan Thank you very much for the P.R. and the kind words! Your project is inspiring and sets a high standard. Regarding the previews not working, I first noticed it recently, even yesterday in #15. The general assumption is that something changed in either GH Actions or Cloudflare, affecting Adrian's custom action setup for Cloudflare Pages. Please bear with this as we await a resolution. Thanks again for your contribution. Please give me a few hours (I'm about to commute) and I'll provide a proper review. |
There was a problem hiding this comment.
Hey, @ieedan
Thanks again for taking the time. Here are some suggestions about implementation direction.
Please let me know what you think.
|
I'll be more available in a few hours and I'll take another look thanks! |
|
I am going to work on this now. Is auto closing on selection not the intended behavior? Thats normally my default and what Vercel is doing here. |
It is. But oddly, it's not working. 🤷 |
Yeah sorry I am fixing that right now. |
|
Everything should be working good now. I had to create a context for the close function since I separated the list so I added a helper for that. |
|
Still uses dialog implementation right now but I can change to the cmdk-sv implementation if needed. |
|
I do think it may still be necessary to have logic for keyboard handling even for 'mobile' users. Sometimes if you are browsing with a smaller screen on a laptop or something it may still be nice to have. |
|
Hey, @ieedan Could you merge the changes from main and see if the previews are working? (You are working off |
Oops next time I'll make a branch |
|
Please don’t worry about wasting my time—every contribution is greatly appreciated. Regarding casing, I'll definitely set up ESLint tomorrow. If you still have time and energy to work on this, please go ahead! |
Issue Tracker
|
|
Okay I think that is everything. FF is so dumb sometimes I swear why do I have to stopProp and preventDef |
|
It'll probably be worth squashing this one for a cleaner history. Sorry about that 😅 |
|
These tweaks are in the spirit of keeping the As for focus and cmd+k, here is a video of what I was seeing before. Screen.Recording.2024-07-26.at.22.00.19.movShould be resolved on my end, would appreciate confirmation that I didn't mess up anything on your end. I'm really sorry for dragging this out longer than it should have. 🙂 |
shyakadavis
changed the title
Super cool project completely blowing mine out of the water!
I was checking it out on mobile and noticed this wasn't implemented yet.
This is a pretty rough implementation but I think it looks decent and does everything Vercel's does. Unfortunately I can't make use of cmdk-sv here because it only works as a modal not with responsive transformation to a drawer. May be worth opening a PR for in the future.
I had to add a
magnifying-glassicon not sure if you are just copy pasting from Vercel but thats what I did.I put the
Commandcomponent in the header to save some complexity of having to setup state to call it but it could be moved if needed (would just add some complexity).