Milestone: Work & About Section #2

geekelo · 2023-04-29T21:24:49Z

Hello,
I am reaching out to request approval to merge the pull request. I would greatly appreciate it if you could take a look at the code.

Things I worked on;

Correct linter errors
Add HTML codes for "Work" section
Add HTML structure for the "About Self" section
Add CSS styles for the "Work" section
Add CSS styles for the "About" Section
Add descriptive comments within codes

Thank you.

V-Blaze

Hi @geekelo ,

While you made a great effort in this project, unfortunately, I cannot proceed to review your code.

Invalid Code Review Request

You have submitted a project, that has a failed linter check, As required by Microverse, All your linter checks must pass before requesting for a review

Also You did great adding a pull request summary, But following the best practice, A pull request summary should be a brief point and list of all the features you added to the current branch I'll greatly advise you edit your pull request description and add an eye-catching summary. 😄 👋 👍

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.

brainconnect93

STATUS: Changes Required ♻️

Hi @geekelo, 👋

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!

Good Points 👍

Descriptive Pull Request title and summary. ✔️
Create the project cards and well aligned✔️
Create the About me section with the tech stack ✔️
Descriptive and professional README file ✔️
Good use of Github flow ✔️

Changes Required

Please check the comments under the review.

Optional suggestions

[Optional] I would kindly advice you limit your usage of div tag. they should used for parent container for easy flex and grid property.

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 on( @brainconnect93 ) 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.

brainconnect93 · 2023-05-01T08:52:45Z

index.html

+          <div class="workHeading">
+            <p class="workTitles defaultFontFamily">Tonic</p>
+	    <div class="workMetadatas defaultFontFamily">
+	      <div class="metaCategories"><p>CANOPY</p></div>


I would kindly suggest you make this part and other part that are of thesame type be inside of an hx tag i.e h2-h6.
I would preferably ask you to make use of an h3 tag rather than the p tag you have there. As this will show that you have follow the project requirement as much as possible.

brainconnect93 · 2023-05-01T08:55:08Z

index.html

+      <!-- about self section -->
+      <div id="aboutSelfSection">
+        <div id="aboutSelfTitle" class="aboutTitles defaultFontFamily">
+          <h1>About Myself</h1> 


According to the requirement we are told to use an hx tag, which you have rightly used. But it is of the best practice to have only one h1 tag in a webpage and you have used that when working with the header section. I would kindly suggest you make use of h2-h6 tag here, preferably h2 tag

brainconnect93 · 2023-05-01T08:59:34Z

index.html

+	    <p class="experienceTitles defaultFontFamily">Languages</p>
+	    <div class="tabIcons"><img src="./media-library/downarrow.png" alt="tab icon" width="" height=""></div>
+	  </div>
+
+	  <!-- lang subtab section -->
+	  <div class="tabBodies">
+	    <ul class="subtabContainers">
+              <li class="subTabs"><img src="./media-library/Ellipse1.png" alt="lang icon" width="" height=""><p class="subTabsTexts defaultFontFamily">JavaScript</p></li>
+	      <li class="subTabs"><img src="./media-library/Ellipse2.png" alt="lang icon" width="" height=""><p class="subTabsTexts defaultFontFamily">HTML</p></li>
+	      <li class="subTabs"><img src="./media-library/Ellipse3.png" alt="lang icon" width="" height=""><p class="subTabsTexts defaultFontFamily">CSS</p></li>
+	    </ul>
+	  </div>
+        <!-- tab 2 -->
+	<div class="experienceTabs inactiveExpTabs">
+          <div class="tabHeads defaultFontFamily">
+	    <p class="experienceTitles">Frameworks</p>
+            <div class="tabIcons"><img src="./media-library/rightarrow.png" alt="tab icon" width="" height=""></div>
+          </div>
+	</div>


As stated in the requirement, I would kindly advice you make The technology categories ("Languages", "Frameworks" and "Skills") are in a ul, each of them in a li. this will show confidence in your ability to follow any given requirement.

mckent05

Hi @geekelo,

STATUS: APPROVED ✔️ ✔️

Your project is complete! There is nothing else to say other than... it's time to merge it
Congratulations and well done! 🎉

Highlights

All linter checks are passing ✔️ ✔️
Your PR is professional 👏 👏
Your project looks good, welldone 👍 👍

Optional suggestions

OPTIONAL - According to the project requirements, please make sure your mobile version is responsible for all screen sizes between 375px and 768px. This would ensure your website fits into all mobile screen and tabs.

Your project:

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.

mckent05 · 2023-05-01T10:47:30Z

index.html

+	    <ul class="subtabContainers tabBodies">
+              <li class="subTabs"><img src="./media-library/Ellipse1.png" alt="lang icon" width="" height=""><p class="subTabsTexts defaultFontFamily">JavaScript</p></li>
+	      <li class="subTabs"><img src="./media-library/Ellipse2.png" alt="lang icon" width="" height=""><p class="subTabsTexts defaultFontFamily">HTML</p></li>
+	      <li class="subTabs"><img src="./media-library/Ellipse3.png" alt="lang icon" width="" height=""><p class="subTabsTexts defaultFontFamily">CSS</p></li>
+	    </ul>


OPTIONAL - Good job following the advice of the previous reviewer, but to ensure your page is properly semantically correct, please put this ul and its children in the first li tag. Please fix this 🙏

Thank you @mckent05

geekelo added 3 commits April 29, 2023 22:19

Add Work section

244e92c

Add styles for Work section

9f73dae

Recorrect style formats

4153ac2

V-Blaze suggested changes Apr 29, 2023

View reviewed changes

geekelo added 3 commits May 1, 2023 07:47

Add Work and About sections

2d15b69

Add styles for Work and About sections

32b0f24

Add media files for project use

e0eddd9

geekelo changed the title ~~Requesting Approval to merge "Main section" into "Master" branch~~ Milestone: Work & About Section May 1, 2023

Update README.md

905eb76

brainconnect93 suggested changes May 1, 2023

View reviewed changes

geekelo added 2 commits May 1, 2023 10:50

Replace excess DIVs with appropriate tags

eb81e22

Update style rules to reflect changes done in HTML

d7ff693

mckent05 approved these changes May 1, 2023

View reviewed changes

geekelo merged commit 5a557a0 into master May 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Milestone: Work & About Section #2

Milestone: Work & About Section #2

geekelo commented Apr 29, 2023 •

edited

Loading

V-Blaze left a comment

brainconnect93 left a comment •

edited by geekelo

Loading

brainconnect93 May 1, 2023

brainconnect93 May 1, 2023

brainconnect93 May 1, 2023

mckent05 left a comment

mckent05 May 1, 2023

geekelo May 1, 2023

Milestone: Work & About Section #2

Milestone: Work & About Section #2

Conversation

geekelo commented Apr 29, 2023 • edited Loading

V-Blaze left a comment

Choose a reason for hiding this comment

Invalid Code Review Request

You have submitted a project, that has a failed linter check, As required by Microverse, All your linter checks must pass before requesting for a review

Also You did great adding a pull request summary, But following the best practice, A pull request summary should be a brief point and list of all the features you added to the current branch I'll greatly advise you edit your pull request description and add an eye-catching summary. 😄 👋 👍

brainconnect93 left a comment • edited by geekelo Loading

Choose a reason for hiding this comment

STATUS: Changes Required ♻️

Hi @geekelo, 👋

Good Points 👍

Changes Required

Optional suggestions

brainconnect93 May 1, 2023

Choose a reason for hiding this comment

brainconnect93 May 1, 2023

Choose a reason for hiding this comment

brainconnect93 May 1, 2023

Choose a reason for hiding this comment

mckent05 left a comment

Choose a reason for hiding this comment

STATUS: APPROVED ✔️ ✔️

Highlights

Optional suggestions

mckent05 May 1, 2023

Choose a reason for hiding this comment

geekelo May 1, 2023

Choose a reason for hiding this comment

geekelo commented Apr 29, 2023 •

edited

Loading

brainconnect93 left a comment •

edited by geekelo

Loading