-
-
Notifications
You must be signed in to change notification settings - Fork 557
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
fix bug: text overlap in search result #1471
fix bug: text overlap in search result #1471
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, harmeetsingh11, for creating this pull request and contributing to LinksHub! 💗
The maintainers will review this Pull Request and provide feedback as soon as possible! 😇
We appreciate your patience and contribution, Keep up the great work! 😀
Hey @rupali-codes, @CBID2, @ujjawaltyagii please review this PR. |
Hmm... for some reason the deployment build failed. @rupali-codes, can you describe the deployment error? I can't see it because I don't have access to your Vercel account. |
@harmeetsingh11 and @CBID2 i'm sorry for being late, your vercel deployment is getting failed because of the following error |
@rupali-codes , @CBID2 I have fixed the error. Please check it. |
Can you post another link to your screenshot video @harmeetsingh11? I'm having trouble visualizing the changes you made. |
Sure @CBID2 . Here it is. Please do let me know, in case this doesn't works. screenshot.video.mp4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall it looks great, lets just improve the texts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@rupali-codes I've made the suggested changes. Please review it. LinksHub.webm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks perfect to me, thanks a lot!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @harmeetsingh11. One of the tests had an error. Fix it and tag me when you're ready for another review.
@CBID2 I've fixed the error. Hope it will work. |
It still failed @harmeetsingh11 |
@CBID2 please check. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fixes
Closes #1218
Changes proposed
Simply display
#Search: Results Not Found
or#Search: Results Found
, and it will change based on the click or further interactions.Files modified/added
components/TopBar/TopBar.tsx
components/Header/Header.tsx
pages/_app.tsx
pages/search.tsx
hooks/ResultsContext.tsx
Screenshots
Before this PR
After this PR
LinksHub.webm
Checklist: