Project Portfolio Desktop version Pull Request #8

Uzair-Manzoor · 2023-06-14T19:51:43Z

Project has been updated to desktop version according to the the given instructions;

added media query for desktop screen resolution
animations added in a animation-transition branch by assigned coding partners as per given instructions
added linters

…into desktop-version

Strangeal

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

Correct use of GitHub Flow ✅
Well-structured files ✅
All linter checks are passing ✅
Descriptive PR title with a great summary ✅

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 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.

Strangeal · 2023-06-14T20:57:31Z

style.css

+.work-section {
+  margin: 80px 0;
+  display: grid;
+  row-gap: 80px;
+  padding: 10px;
+  justify-items: center;
+}


[OPTIONAL] Kindly consider adding a margin on the left and right side of your work card container to help make the card look a bit more stunning on 600px screens and above.

Strangeal · 2023-06-14T20:57:59Z

style.css

+.work-preview {
+  display: flex;
+  align-self: center;
+  flex-direction: column;
+  justify-self: center;
+  overflow: hidden;
+}


Please note that your website supposes to be fully responsive across all screen sizes (mobile, tablet, and desktop). In other to do that and also create a good user experience kindly consider the following suggestions to make your website more responsive:

Your work cards images are not responsive as expected on 400px screen sizes and above, To fix this issue kindly consider setting a width: 100% on the class name work-preview as this will help make your image maintain it's full width based of the screen size resulting in a responsive UI.

Figma design Your design

Strangeal · 2023-06-14T21:02:58Z

style.css

+.work-preview:hover .project-img {
+  transform: scale(1.4);
+}


Kindly consider reducing the scale of the hover effect you have on your image. I love the animation but I think it would be much better if it doesn't make the user kindly wrack out 😊 as that will create a bad user experience and that's something we don't need. You can reduce it to maybe (1.1) so it doesn't really look too much.

Strangeal · 2023-06-14T21:17:37Z

style.css

  .work-card {
-    background: #fff;
-    border-radius: 10px;
-    min-height: 3em;
-    border: 1px solid #c6c6c6;
-    padding: 20px 15px;
-    display: grid;
+    display: flex;
+    flex-direction: row;
  }


Why don't you use grid instead of flex to display the content of your cards in a row direction? I think using grid is going to be much better so kindly consider using it so that both your card image and text will have equal fractions of the main container.
HINT: You can give it a display: grid and grid-template-columns: 1fr 1fr. This way both children of the card container will have equal fractions and will also help make your website even more responsive. See below for more details:

Figma design Your design

Strangeal · 2023-06-14T21:28:50Z

style.css

  .work-preview {
-    display: flex;
-    align-self: center;
-    flex-direction: column;
-    justify-self: center;
-    overflow: hidden;
+    padding-left: 3%;
+    height: 500px;
+    width: 50%;
+    justify-content: center;
  }


Please make sure that your design looks as close to the Figma design as possible. Looking at your work card I can see you have empty space at the top and bottom of the card. This is because you have specified a height of 50% for the card. To fix this issue so your design looks like that of the Figma design and also help create a good user experience please consider the following hints:
HINT: Remove the height: 50% and increase the width to 100%. This will make your image cover the full width of its container. See below for more details:

Figma design Your design

Strangeal · 2023-06-14T21:37:14Z

style.css

+.work-detail {
+  align-self: center;
+  padding: 10px;
+}


To make your work details align just like your image from top to bottom, Kindly consider aligning your details start instead of center as that will make your design look like that of the Figma design and also help make your design look professional. 😊

@Strangeal Thanks a lot for the Hints 👍. Changes have been made exact accordingly.

skyv26

Hi @Uzair-Manzoor,

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

STATUS: APPROVED 🟢

Highlights

Good PR title ✅
Descriptive PR description ✅
Implement the features ✅
Professional README template ✅
Linters are green ✅
Correct GitHub workflow ✅
Meaningful Github commits ✅

Optional suggestions

Every comment with the [OPTIONAL] prefix won't stop the approval of this PR. However, I strongly recommend you 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.

⚠️ WARNING ⚠️

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.

Have any doubt ❓

Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag @skyv26 in your question so I can receive the notification. You can also follow me on the below platforms

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.

Uzair-Manzoor added 5 commits June 15, 2023 00:45

update html file

6b81031

update css file

651649c

Merge branch 'main' into desktop-version

a4e2638

resolve css linter errors

0769f7e

Merge branch 'desktop-version' of github.com:Uzair-Manzoor/Portfolio …

9f0a513

…into desktop-version

Uzair-Manzoor temporarily deployed to github-pages June 14, 2023 20:13 — with GitHub Pages Inactive

update html file

7e12b7b

Uzair-Manzoor temporarily deployed to github-pages June 14, 2023 20:16 — with GitHub Pages Inactive

Strangeal suggested changes Jun 14, 2023

View reviewed changes

Uzair-Manzoor added 2 commits June 15, 2023 11:37

update html file

3a5d7de

update css file

6800e5a

Uzair-Manzoor temporarily deployed to github-pages June 15, 2023 06:39 — with GitHub Pages Inactive

update html file

265d34a

Uzair-Manzoor temporarily deployed to github-pages June 15, 2023 06:40 — with GitHub Pages Inactive

resolved css linter errors

7824c78

Uzair-Manzoor temporarily deployed to github-pages June 15, 2023 06:43 — with GitHub Pages Inactive

skyv26 approved these changes Jun 15, 2023

View reviewed changes

update css file

c98bec5

Uzair-Manzoor temporarily deployed to github-pages June 15, 2023 07:54 — with GitHub Pages Inactive

Uzair-Manzoor merged commit b35cd29 into main Jun 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Project Portfolio Desktop version Pull Request #8

Project Portfolio Desktop version Pull Request #8

Uzair-Manzoor commented Jun 14, 2023

Strangeal left a comment

Strangeal Jun 14, 2023

Strangeal Jun 14, 2023 •

edited by Uzair-Manzoor

Loading

Strangeal Jun 14, 2023 •

edited by Uzair-Manzoor

Loading

Strangeal Jun 14, 2023 •

edited by Uzair-Manzoor

Loading

Strangeal Jun 14, 2023 •

edited by Uzair-Manzoor

Loading

Strangeal Jun 14, 2023 •

edited by Uzair-Manzoor

Loading

Uzair-Manzoor Jun 15, 2023

skyv26 left a comment

Project Portfolio Desktop version Pull Request #8

Project Portfolio Desktop version Pull Request #8

Conversation

Uzair-Manzoor commented Jun 14, 2023

Strangeal left a comment

Choose a reason for hiding this comment

Highlights

Required Changes ♻️

Optional suggestions

Strangeal Jun 14, 2023

Choose a reason for hiding this comment

Strangeal Jun 14, 2023 • edited by Uzair-Manzoor Loading

Choose a reason for hiding this comment

Strangeal Jun 14, 2023 • edited by Uzair-Manzoor Loading

Choose a reason for hiding this comment

Strangeal Jun 14, 2023 • edited by Uzair-Manzoor Loading

Choose a reason for hiding this comment

Strangeal Jun 14, 2023 • edited by Uzair-Manzoor Loading

Choose a reason for hiding this comment

Strangeal Jun 14, 2023 • edited by Uzair-Manzoor Loading

Choose a reason for hiding this comment

Uzair-Manzoor Jun 15, 2023

Choose a reason for hiding this comment

skyv26 left a comment

Choose a reason for hiding this comment

Hi @Uzair-Manzoor,

STATUS: APPROVED 🟢

Highlights

Optional suggestions

⚠️ WARNING ⚠️

Have any doubt ❓

Strangeal Jun 14, 2023 •

edited by Uzair-Manzoor

Loading

Strangeal Jun 14, 2023 •

edited by Uzair-Manzoor

Loading

Strangeal Jun 14, 2023 •

edited by Uzair-Manzoor

Loading

Strangeal Jun 14, 2023 •

edited by Uzair-Manzoor

Loading

Strangeal Jun 14, 2023 •

edited by Uzair-Manzoor

Loading