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

feat: implement search highlight #205

Merged
merged 4 commits into from
Oct 29, 2024

Conversation

ansh-saini
Copy link
Contributor

Fixes #96

I'm using the browser built-in mark tag to achieve the highlight. This will ensure standard behaviour on all operating systems.

Here's a screenshot of how it looks:
image

@ansh-saini
Copy link
Contributor Author

@snigdha-kansal Review please? :)

@ansh-saini
Copy link
Contributor Author

@snigdha-kansal Did you get a chance to review this?

Copy link
Contributor

@snigdha-kansal snigdha-kansal left a comment

Choose a reason for hiding this comment

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

@ansh-saini sorry for the delay on this

one thing i observed: entering certain characters such as '\' '+' '*' '(' ')' or '?' in the search bar causes the app to crash
you can either reproduce it locally or at deploy preview

other than that, it looks great to me! thanks so much for working on this 💯
appreciate you going beyond to improve naming and formatting where possible!

Yoda Very Good

@ansh-saini
Copy link
Contributor Author

Thank you for the review @snigdha-kansal
You found a great bug! I've fixed it.

@snigdha-kansal
Copy link
Contributor

snigdha-kansal commented Oct 28, 2024

@ansh-saini It looks great, thanks!
One last thing: Can we remove match support for if user enters just a single spacebar?

image

@ansh-saini
Copy link
Contributor Author

Another great find @snigdha-kansal.
Thanks, I've resolved it.

Copy link
Contributor

@snigdha-kansal snigdha-kansal left a comment

Choose a reason for hiding this comment

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

:shipit:

@ansh-saini
Copy link
Contributor Author

ansh-saini commented Oct 29, 2024

Hey @snigdha-kansal Thank you for the approval.
Can you please also add the hacktoberfest-accepted label on this PR?

Also, just curious as to why you've approved this PR but not merged it. Is there something blocking the merge?

@snigdha-kansal
Copy link
Contributor

@ansh-saini just added the label
oh i just thought you will like to merge it 😂 i can merge!

@snigdha-kansal snigdha-kansal merged commit 0512e66 into uclaacm:main Oct 29, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bolding Sections of Project That Contain Searchbar Contents
2 participants