-
Notifications
You must be signed in to change notification settings - Fork 0
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
Portfolio mobile menu #10
Conversation
…hammad/my-portfolio into portfolio-mobile-menu
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 @the-faizmohammad, @binyamolango, @MohammadYaser,
While you made a great effort in this project, unfortunately, I cannot proceed to review your code.
Invalid Code Review Request
Invalid Code Review Request🛑
- Kindly update your linters.yml file for this project to include ESLinters. In the root project, you have a folder named .github. Inside this one, you have another folder named workflows. And inside the workflows folder, you have a file named linters.yml. Inside this file, you need to update the linters. The linters needed for this project can be found here;
Desp | Images |
---|---|
Actions without ESLint | |
Actions with ESLint |
Your Code Review Request will be marked as invalid in your Dashboard, so please submit a new one once you are ready 🙏
Cheers and Happy coding!👏👏👏
Feel free to leave any questions or comments in the PR thread if something is not 100% clear. Please, remember to tag me in your question so I can receive the notification.
Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the first review, either valid or invalid unless it is requested otherwise.
Invalid Code Review Request does not count into the code reviews limit.
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.
Status: Approved! 🟢
Your project is complete! There is nothing else to say other than... it's time to merge it
Congratulations! 🎉 🎉
Highlights 🎉
✅ Professional and customized README. 🤓
✅ Linters tests passing. 👍
✅ Correct use of GitHub Flow. 🙌
✅ Great job using JavaScript to show your menu. 💯
Optional suggestions 💭
Every comment with the [OPTIONAL] prefix is not crucial enough to stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better.
Cheers and Happy coding!👏👏👏
Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag me @taldr27 in your question so I can receive the notification.
If you need any further help or have any questions, feel free to reach out to me on Slack as Diego Garcia.🎯
As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.
.mobile-menu--show { | ||
display: flex; | ||
flex-direction: column; | ||
position: absolute; | ||
top: 0; | ||
left: 0; | ||
padding: 1em 0.75em; | ||
width: 100vw; | ||
height: 100vh; | ||
color: #fff; | ||
background: rgba(96, 112, 255, 0.9); | ||
font-family: "Poppins", sans-serif; | ||
font-weight: 600; | ||
font-size: 2rem; | ||
line-height: 44px; | ||
backdrop-filter: blur(4px); | ||
} |
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.
-
[OPTIONAL] You've done a fantastic job with your mobile menu! I have a small optional suggestion that could enhance its professional appearance. Would you consider adding a
position fixed
to the overlay when the menu is opened? Currently, users are able to scroll down while the mobile menu remains at the top, and implementing this change would address that issue.scrnli_6_28_2023_7-07-14.AM.webm
Pull request Summary
This pull request enhances the existing code related to the mobile menu functionality.
The changes ensure that the mobile menu appears when the hamburger button is clicked and disappears when the close button (X) is clicked or when any of the mobile menu options are clicked.
Additionally, the code correctly handles the display of the corresponding part of the page when a mobile menu option is selected.
Updated the READme file.
The following modifications were made:
Consolidated the event listeners for opening and closing the mobile menu into a single function called toggleMobileMenu().
Attached the toggleMobileMenu() function as the event handler for both the hamburger button (navShow) and the close button (navHide).
Added an event listener to each mobile menu option (mobMenuLinks) to prevent the default behavior and ensure the mobile menu disappears when an option is clicked.
Ensured that the appropriate part of the page is displayed when a mobile menu option is selected.