Modal Pop Up Feature #11
Conversation
Baayeh
commented
Sep 8, 2022
Baayeh
commented
- Created an array of project objects in the index.js file.
- Dynamically injecting the individual project into the HTML.
- Designed the modal pop-up with an overlay.
- Dynamically displayed the individual detail of each project in the modal when the "see project" button is clicked.
There was a problem hiding this comment.
Hi @Baayeh ,
Good job so far! 👏 👏
There are some issues that you still need to work on to go to the next project but you are almost there! 🏃♂️ 🏃♂️
Highlights 💪
- No linter errors
- correct flow is used
- good PR
Required Changes ♻️
Check the comments under the review.
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 @BertrandConxy 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.
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.
.github/workflows/linters.yml
Outdated
| @@ -29,12 +29,12 @@ jobs: | |||
| - name: Setup Webhint | |||
| run: | | |||
| npm install --save-dev hint@7.x | |||
| [ -f .hintrc ] || wget https://raw.githubusercontent.com/microverseinc/linters-config/master/html-css-js/.hintrc | |||
| [ -f .hintrc ] || wget https://raw.githubusercontent.com/microverseinc/linters-config/master/html-css/.hintrc | |||
There was a problem hiding this comment.
- It looks like you forgot to set up the eslint checks on GitHub. Normally, the linter checks should be 5 but now they are four. That means eslint action check is not set up 😃. But don't worry, it is just a second to finish it 😎. Please update your
linters.ymlfile by using this one 👉 HERE because it includes all the rules to run all 5 necessary linter checks for this project 😉
styles.css
Outdated
| div#proj-overlay { | ||
| position: fixed; | ||
| width: 100%; | ||
| height: 100vh; | ||
| background-color: #3a4a4290; | ||
| top: 0; | ||
| left: 0; | ||
| right: 0; | ||
| bottom: 0; | ||
| z-index: 10; | ||
| display: none; | ||
| overflow-y: auto; | ||
| } | ||
|
|
||
| .proj-name { | ||
| grid-area: proj-name; | ||
| font-size: 32px; | ||
| font-weight: 700; | ||
| color: #172b4d; | ||
| } |
There was a problem hiding this comment.
- Good job so far 👏. In addition to what you have done, I would suggest you adjust the styles of the popup window in order to fit in the window screen without needing to scroll down to view all the content. This improves the user experience.
This is how currently it looks 👇
I really liked the way it looks on the mobile screens 😃 💯 and It will be good if it looks great on the desktop screens too.
There was a problem hiding this comment.
Hi @Baayeh,
Your project is complete! There is nothing else to say other than... it's time to merge it 
Congratulations! 🎉
To Highlight 💯
- Correct use of GitFlow ✔️
- Descriptive commit messages ✔️
- Professional looking pull request ✔️
- Your readme looks professional ✔️
- Good overall design ✔️
- Your code is clean and well documented ✔️
Approved 👌 👌 👌
Optional suggestions
Every comment with the [OPTIONAL] prefix won't stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better. Some of them were simply missed by the previous reviewer and addressing them will really improve your application.
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.
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.
Fixed the below errors styles.css 641:1 ✖ Expected empty line before comment comment-empty-line-before 949:2 ✖ Unexpected whitespace at end of line no-eol-whitespace
styles.css 950:2 ✖ Unexpected whitespace at end of line no-eol-whitespace
