-
Notifications
You must be signed in to change notification settings - Fork 33
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 Home link placement in the header for better accessibility #76
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
review please author |
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.
Hey @Chirag-S-Kotian thank you very much!
IF YOU NEED ANY OTHER HELP PLEASE LET ME KNOW. |
Hey @Chirag-S-Kotian I left a comment to improve UX. Feel free to push the new commit here without closing the Pull Request. |
i fixed the link problem. |
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.
Could you run yarn prettier
and push the changes?
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.
Hey! I see that yarn prettier:write
worked well. Could you remove the package-lock.json
and yarn.lock
files. Any package version should be updated here.
apps/web-app/package.json
Outdated
@@ -16,10 +16,10 @@ | |||
"@emotion/react": "^11.11.3", | |||
"@emotion/styled": "^11.11.0", | |||
"@semaphore-protocol/core": "4.3.1", | |||
"@semaphore-protocol/data": "4.3.1", |
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.
There is no need to update these versions.
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.
Could you please tell me how to remove package.json and yarn.lock , should I delete from my repo?
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.
Hey!
You can delete the package-lock.json
file from your repo.
Regarding the yarn.lock
file, if you keep the current versions of all the packages, everything should be fine and you won't need to remove this file. To keep the current versions of all the packages you need to have this: "@semaphore-protocol/data": "4.3.1"
instead of this: "@semaphore-protocol/data": "^4.4.1"
and "next": "14.1.0"
instead of "next": "^14.2.15"
in the package.json
file.
Please let me know if you have any other questions.
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.
okay i removed both and updated package.json file
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.
Hey! Great! Thank you! Please, could you create an empty yarn.lock
file in the root of the project and run the command yarn
and push the changes? The yarn.lock
file should still be in the repo but this PR shouldn't modify it.
After that, I think the PR is ready.
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.
i did it anyway and pushed same yarn.lock which is present in boilerplate repo
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.
Hey! The new yarn.lock
file is almost correct. If you run yarn
, it will be updated and you can push the change. The change will be just adding a line at the end of the file.
The yarn.lock
file should not be part of these files after you push the change: https://github.com/semaphore-protocol/boilerplate/pull/76/files
Right now it's there: https://github.com/semaphore-protocol/boilerplate/pull/76/files#diff-51e4f558fae534656963876761c95b83b6ef5da5103c4adef6768219ed76c2de
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.
okay changes are complete.
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.
Hey! Thank you @Chirag-S-Kotian!
I added two suggestions. Please push the changes when you update those versions and run yarn
.
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.
Hey @Chirag-S-Kotian thank you very much!
Congrats, your important contribution to this open-source project has earned you a GitPOAP! GitPOAP: 2024 Semaphore Contributor: Head to gitpoap.io & connect your GitHub account to mint! Learn more about GitPOAPs here. |
thank you for your time, if you have anything other issue please let me know. |
Description
This PR updates the Home link functionality and layout as per the review feedback. The goal is to ensure the Home link is easily accessible and seamlessly integrated into the header, improving the overall user experience.
Updates:
"/"
) without any issues.Feedback Incorporated:
Screenshots
Checklist: