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

Highlight Support #425

Merged
merged 20 commits into from
Oct 10, 2023
Merged

Highlight Support #425

merged 20 commits into from
Oct 10, 2023

Conversation

Jothsa
Copy link
Contributor

@Jothsa Jothsa commented Sep 15, 2023

This should work. All the tests pass. A few things I'm not sure about:

  • The user can customize the highlight query param name, but it has to be set in both the PagefindUI script and PagefindHIghlight script. This is kind of annoying, but I don't see another way.
  • I have provided optional styles, but they're so minimal that I don't know if it's worth it. They're basically the browser default styles. Maybe it would be better to use some of the --pagefind css vars?

I'll try to add docs later Added some docs

@bglw
Copy link
Contributor

bglw commented Sep 15, 2023

Amazing, thank you for putting this work in!

I have a bit of a busy week/weekend, but I'll crack into reviewing this next week ❤️

@bglw
Copy link
Contributor

bglw commented Oct 2, 2023

Sorry to leave you hanging! Had to deal with some tasks that were put aside while working on the 1.0 release. Coming back around to new PRs and issues in this repo soon.

@bglw
Copy link
Contributor

bglw commented Oct 10, 2023

Apologies again for the delay — implementation looks good! I'll make a few docs tweaks before the next release, but I can tackle that after merge.

Thanks for the contribution! ❤️

@bglw bglw enabled auto-merge October 10, 2023 04:38
@bglw bglw merged commit cfcaec4 into CloudCannon:main Oct 10, 2023
4 checks passed
@Jothsa Jothsa deleted the highlight-script branch October 12, 2023 18:28
@bglw
Copy link
Contributor

bglw commented Nov 16, 2023

Finally got this out the door in 1.0.4 🙂 Made a few changes, you'll see some notes in #502

Thanks again!

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.

2 participants