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 UX for client-side search #470

Merged
merged 38 commits into from
Sep 24, 2024

Conversation

agoose77
Copy link
Collaborator

@agoose77 agoose77 commented Sep 13, 2024

This PR is a rough stab at a frontend component for client-side search. There are already some sharp edges to rework:

  • Remove reference to @myst-theme/search in @myst-theme/common once mystmd exports myst-spec-ext with the latest tyeps
  • Handle pagination or scrolling
  • Improve highlight renderer to e.g. reduce ellipsis 🤣
  • Add "context" e.g. current doc/section and highlight for each entry
  • Think about whether this can/should be generalised for a server-side search
2024-09-23.19-51-13.mp4

Note

To test this PR:

  1. Build 🔍 Add client-side search index generation mystmd#1530 and run myst start --headless
  2. Run theme:book as usual

Copy link

changeset-bot bot commented Sep 13, 2024

🦋 Changeset detected

Latest commit: 4fd5216

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 12 packages
Name Type
@myst-theme/site Patch
@myst-theme/book Patch
@myst-theme/providers Patch
@myst-theme/frontmatter Patch
@myst-theme/diagrams Patch
@myst-theme/jupyter Patch
@myst-theme/styles Patch
@myst-theme/common Patch
@myst-theme/icons Patch
@myst-theme/article Patch
myst-to-react Patch
myst-demo Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@agoose77 agoose77 marked this pull request as draft September 13, 2024 12:18
@agoose77
Copy link
Collaborator Author

@stevejpurves @rowanc1 input always welcome, but I'll keep chipping away at this.

@agoose77 agoose77 changed the base branch from main to agoose77/feat-search-json September 13, 2024 12:19
@agoose77 agoose77 changed the title 🔍 💥 Add UX for client-side search 🔍 💥 Add UX for client-side search (2 of 2) Sep 13, 2024
@agoose77
Copy link
Collaborator Author

Phew, made some good progress! My goal here is to build out "an approach", and then we can iterate on the design.

@agoose77 agoose77 force-pushed the agoose77/feat-add-client-side-search branch from 343703e to 35243a3 Compare September 18, 2024 10:47
Copy link
Collaborator

@rowanc1 rowanc1 left a comment

Choose a reason for hiding this comment

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

In progress comments!

packages/site/src/components/Navigation/Search.tsx Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@agoose77 agoose77 marked this pull request as ready for review September 20, 2024 10:41
Copy link
Contributor

@choldgraf choldgraf left a comment

Choose a reason for hiding this comment

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

From a user's perspective, based on the two videos above, I think that this is ready to ship and consider it an alpha-level feature. I like the UI responsiveness, and the results modal is a great start - +1 to mimicking the UI of Algolia since they've done a lot of UI research already.

I couldn't test this locally (tried to get this theme building for 15 minutes but couldn't figure it out) but my feeling is that we should try to get this into the hands of users for feedback sooner than later, it already feels ready to me from a functionality perspective.

Is there any major technical blocker that we need to work out first? If not, my vote is to ship this and iterate when we get feedback. I'm happy to start trying it out a bit more.

@agoose77 agoose77 force-pushed the agoose77/feat-add-client-side-search branch from cac0a98 to 4cf43ac Compare September 23, 2024 18:26
@agoose77 agoose77 changed the base branch from agoose77/feat-search-json to main September 24, 2024 16:17
@rowanc1 rowanc1 changed the title 🔍 💥 Add UX for client-side search (2 of 2) 🔍 💥 Add UX for client-side search Sep 24, 2024
@rowanc1 rowanc1 merged commit 336e47b into main Sep 24, 2024
@rowanc1 rowanc1 deleted the agoose77/feat-add-client-side-search branch September 24, 2024 16:18
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