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

[MPT-44] Change search result highlighting #653

Merged
merged 7 commits into from
Nov 15, 2023

Conversation

jeffzy15
Copy link
Contributor

I have changed the search result highlighting from italics to bold.

[MPT-44] issue screenshot

@jeffzy15 jeffzy15 requested a review from wei2912 October 20, 2023 08:29
@jeffzy15 jeffzy15 self-assigned this Oct 20, 2023
@netlify
Copy link

netlify bot commented Oct 20, 2023

Deploy Preview for tender-meitner-99286b ready!

Name Link
🔨 Latest commit 65dc9de
🔍 Latest deploy log https://app.netlify.com/sites/tender-meitner-99286b/deploys/6550863ede39b600089dd9c6
😎 Deploy Preview https://deploy-preview-653--tender-meitner-99286b.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@wei2912
Copy link
Member

wei2912 commented Oct 20, 2023

Think the highlighting should be of similar style to this:

#644 (comment)

Rather than simply applying bold or italics, which doesn't really stand out.

Copy link
Member

@wei2912 wei2912 left a comment

Choose a reason for hiding this comment

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

See above comment. Note that your solution should avoid replacing the em tags with strong since em is meant to semantically represent emphasis (which can be styled in different ways, e.g. bold/italics/underline/highlighting) rather than styling the text specifically to be bold.

@jeffzy15
Copy link
Contributor Author

I have changed the highlighting of the search result to be of similar style to the image provided in #644.

Screenshot 2023-10-21 at 3 22 52 PM

@jeffzy15
Copy link
Contributor Author

I used dangerouslySetInnerHTML= {{ __html: displayName.replace( /<em[^>]*>/g, '<em style="background-color: transparent;">'), }} to remove the background colour styling for the search result in the expanded card view as there were 2 overlapping background colours in the expanded card view.

Screenshot 2023-10-21 at 3 48 18 PM

@jeffzy15 jeffzy15 requested a review from wei2912 October 21, 2023 07:49
Copy link
Member

@wei2912 wei2912 left a comment

Choose a reason for hiding this comment

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

Have a few more questions about the code, otherwise LGTM.

components/ResultView.tsx Outdated Show resolved Hide resolved
components/ResultView.tsx Outdated Show resolved Hide resolved
components/ResultViewList.tsx Show resolved Hide resolved
@jeffzy15 jeffzy15 requested a review from wei2912 November 12, 2023 08:18
@wei2912 wei2912 merged commit 4571e05 into AdvisorySG:multi-page Nov 15, 2023
7 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.

[MPT-44] Mentors: Search result highlighting is displayed as italics
2 participants