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

Adding local search with @cmfcmf-plugin #78

Merged
merged 16 commits into from
Sep 13, 2024

Conversation

FilipHarald
Copy link
Contributor

A first solution to #6

Copy link

vercel bot commented Aug 13, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
scaffold-eth-2-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 12, 2024 4:03pm

@FilipHarald
Copy link
Contributor Author

Example:

simplescreenrecorder-2024-08-13_16.03.37.mp4

@FilipHarald
Copy link
Contributor Author

Used wrong branch. Will leave this PR in draft until #76 is done.

Copy link
Collaborator

@Pabl0cks Pabl0cks left a comment

Choose a reason for hiding this comment

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

Thanks Filip for tackling this! 🙌 It's looking good to me! It's a basic search but I think it gets the job done.

In terms of UI/UX has a couple of things that could be better, but they are nitpicks and probably it's not worth to invest the time in try to tweak the css from the plugin.

I've only pushed a nitpick in README (014a1aa) to try to make it more readable and a config setting (decbb50) to show category context in the search results modal:

Before After
Before configuration change After configuration change

@technophile-04
Copy link
Collaborator

Hey thanks @FilipHarald!! for tacking it! Looks good to me!

Did you try using easyops-cn/docusaurus-search-local? It seems a forked version of cmcf/search-local but with more stars and also some nitpicky good feature which were missing in cmcf/search-local example cmd + k to focus on search bar and also the search bar looks complete as compared to cmcf one also it seems to give better customisation options incase we need.

could you also create an PR with easyops-cn/docusaurus-search-local? I think it should be easy? since it seems like direct replacement to cmcf loca search?

@FilipHarald
Copy link
Contributor Author

@technophile-04 , yup, I made a low-effort try and didn't get it to work. I wrote that here: #6 (comment) .

But I didn't look into it more then since I didn't know if it was going to be used. But I can do that now during the coming days 👍 . I will take @Pabl0cks improvements into account as well.

@technophile-04
Copy link
Collaborator

Hey @FilipHarald any updates on this? Also happy to merge this as it is! I think it gets the job done for now and we can always update later.

A small nitpick(if it gets hard we could merge this as it is) but:

Can me make this Search bar a bit bigger?

Screenshot 2024-09-12 at 8 46 37 PM

Seem like it is too much cramped in

And another ctrl + k or some keyboard shortcut to open it up.

I would say don't waste lot of time on it, we could always iterate later but would be great if we get them working!

@FilipHarald
Copy link
Contributor Author

Hello, sorry, no other things got in the way. I'll have a short look now on ctrl+k and sizing.

@FilipHarald
Copy link
Contributor Author

Shortcut-key seems to be an issue currently: cmfcmf/docusaurus-search-local#50

@FilipHarald
Copy link
Contributor Author

@technophile-04 to be honest I kind on like having the search bar small, since you are not typing in it there anyway. But I've added width to it now. 👍

@technophile-04
Copy link
Collaborator

@technophile-04 to be honest I kind on like having the search bar small, since you are not typing in it there anyway. But I've added width to it now. 👍

Yup agree, it breaks on phone too :(

Screenshot 2024-09-12 at 9 20 11 PM

Let's revert that commit, we could update the styles later 🙌

@technophile-04
Copy link
Collaborator

Thanks @FilipHarald!! Lets keep iterating on this!!

@technophile-04 technophile-04 merged commit f29fa81 into scaffold-eth:main Sep 13, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants