Skip to content
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

release v.1.0 #1

Merged
merged 19 commits into from Nov 13, 2019
Merged

release v.1.0 #1

merged 19 commits into from Nov 13, 2019

Conversation

mekinus
Copy link
Owner

@mekinus mekinus commented Nov 11, 2019

No description provided.

Repository owner deleted a comment from stickler-ci Nov 11, 2019
Repository owner deleted a comment from stickler-ci Nov 11, 2019
Repository owner deleted a comment from stickler-ci Nov 11, 2019
Repository owner deleted a comment from stickler-ci Nov 11, 2019
Repository owner deleted a comment from stickler-ci Nov 11, 2019
Repository owner deleted a comment from stickler-ci Nov 11, 2019
Repository owner deleted a comment from stickler-ci Nov 11, 2019
Repository owner deleted a comment from stickler-ci Nov 11, 2019
Repository owner deleted a comment from stickler-ci Nov 11, 2019
Repository owner deleted a comment from stickler-ci Nov 11, 2019
Repository owner deleted a comment from stickler-ci Nov 11, 2019
Repository owner deleted a comment from stickler-ci Nov 11, 2019
Repository owner deleted a comment from stickler-ci Nov 11, 2019
Repository owner deleted a comment from stickler-ci Nov 11, 2019
Copy link

@ebukaume ebukaume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes Required 🛑

Hi @mekinus 👋

Nice job. You are doing quite well so far. Let's get a few things in order:

  • Your repo does not have a descriptive README. This is a must for all our projects
  • Stickler is failing; please find the errors and fix them.
  • Remember this project is about building with background and gradient; thus the hero image is expected to be a background.
  • The bottom part should contain four elements which are links (to other articles or #).

Kindly submit another code review request after making those fixes

Happy Coding ✌️
Ebuka Umeokonkwo
Github | Linkedin | Slack | Twitter

css/styles.css Outdated
right: 55px;
width: 90%;
display: flex;
background: linear-gradient(to top, #757575, #5F635F, #7D7D7D, #7C807C);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected "#5F635F" to be "#5f635f" (color-hex-case) [error]
Expected "#7D7D7D" to be "#7d7d7d" (color-hex-case) [error]
Expected "#7C807C" to be "#7c807c" (color-hex-case) [error]

css/styles.css Outdated
.nav-item {
margin-top: 10px;
flex: 1;
color: #F5F5F5;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected "#F5F5F5" to be "#f5f5f5" (color-hex-case) [error]

css/styles.css Outdated
}

.nav-item:hover {
background: linear-gradient(to right, #7C807C, #5F635F);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected "#7C807C" to be "#7c807c" (color-hex-case) [error]
Expected "#5F635F" to be "#5f635f" (color-hex-case) [error]

css/styles.css Outdated
}

.custom {
border: 1px solid linear-gradient(to top, #757575, #5F635F, #7D7D7D, #7C807C);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected "#5F635F" to be "#5f635f" (color-hex-case) [error]
Expected "#7D7D7D" to be "#7d7d7d" (color-hex-case) [error]
Expected "#7C807C" to be "#7c807c" (color-hex-case) [error]

css/styles.css Outdated
.custom {
border: 1px solid linear-gradient(to top, #757575, #5F635F, #7D7D7D, #7C807C);
border-radius: 10px;
background: linear-gradient(to top, #757575, #5F635F, #7D7D7D, #7C807C);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected "#5F635F" to be "#5f635f" (color-hex-case) [error]
Expected "#7D7D7D" to be "#7d7d7d" (color-hex-case) [error]
Expected "#7C807C" to be "#7c807c" (color-hex-case) [error]

css/styles.css Outdated
}

.nav-bar {
padding-left: 0px;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected unit (length-zero-no-unit) [error]

css/styles.css Show resolved Hide resolved
css/styles.css Outdated
color: #F5F5F5;
border-right: 1px solid #757575;
font-weight: bold;
font-family: "Arial";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected missing generic font family (font-family-no-missing-generic-family-keyword) [error]

css/styles.css Outdated
flex: 1;
}

footer h6 span a {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected selector "footer h6 span a" to come before selector ".link-item a" (no-descending-specificity) [error]
Expected selector "footer h6 span a" to come before selector ".link-item a:hover" (no-descending-specificity) [error]

css/styles.css Outdated
color: #00A8E3;
}

footer h6 span a:hover {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected selector "footer h6 span a:hover" to come before selector ".link-item a:hover" (no-descending-specificity) [error]

css/styles.css Outdated
}

.border {
border-right: 1px solid #cccccc;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected "#cccccc" to be "#ccc" (color-hex-length) [error]

css/styles.css Outdated
margin-right: 20px;
}

.link-item a {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected selector ".link-item a" to come before selector "footer h6 span a:hover" (no-descending-specificity) [error]

css/styles.css Outdated

.link-item a {
text-decoration: none;
color: #00A8E3;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected "#00A8E3" to be "#00a8e3" (color-hex-case) [error]

Copy link

@ebukaume ebukaume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes Required

Hi @mekinus

Nice job. You are doing quite well so far. Let's get a few things in order:

  • Your repo does not have a descriptive README. This is a must for all our projects
  • Stickler is failing; please find the errors and fix them.
  • Remember this project is about building with background and gradient; thus the hero image is expected to be a background.
  • The bottom part should contain four elements which are links (to other articles or #).

Kindly submit another code review request after making those fixes

Happy Coding
Ebuka Umeokonkwo
Github | Linkedin | Slack | Twitter

@mekinus please refer to this. Please submit another code review request only after making the requested changes.

If you need assistance, you can reach me on slack.

Happy Coding ✌️
Ebuka Umeokonkwo
Github | Linkedin | Slack | Twitter

@mekinus
Copy link
Owner Author

mekinus commented Nov 11, 2019

Fix all those issues, descriptive README file is at master branch

Copy link

@ebukaume ebukaume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes Required!

Nice work making some of those changes. You are almost there.

Your stickler is still showing errors.
review-bad-lint

Please get it to turn green by correcting all the errors being pointed out.
review-good-lint

Submit another code review after achieving this

Happy Coding ✌️
Ebuka Umeokonkwo
Github | Linkedin | Slack | Twitter

Copy link

@Redvanisation Redvanisation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes required 🛑

Hello @mekinus 🌟

Great work on this project, but we need to fix the issues below before moving on ⬇️

  • Below is how the project is looking on my screen, please fix that and make it look like the original page:

Screen Shot 2019-11-11 at 6 43 26 PM

After making the required changes please submit a new code review request

Good luck! 🍀

Copy link

@KaushikShivam KaushikShivam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Status :Changes Required

Hello @mekinus 👋

Good job implementing the changes but you have not finished all the required changes requested by previous TSEs. Please Fix them before making another review request.

Also, as @Redvanisation, the UI looks off on my screen as well. Refer the screenshot.
Screen Shot 2019-11-12 at 5 45 04 PM

That's all!

Make these changes and submit for a code review, again
If you have any questions, let me know here or on slack and I will be glad to clarify with you. You can also reach me and connect with me on my social media or email.

Happy coding! 😄
Shivam Kaushik
Github Twitter Linkedin

@mekinus
Copy link
Owner Author

mekinus commented Nov 12, 2019

Status :Changes Required

Hello @mekinus 👋

Good job implementing the changes but you have not finished all the required changes requested by previous TSEs. Please Fix them before making another review request.

Also, as @Redvanisation, the UI looks off on my screen as well. Refer the screenshot.
Screen Shot 2019-11-12 at 5 45 04 PM

That's all!

Make these changes and submit for a code review, again
If you have any questions, let me know here or on slack and I will be glad to clarify with you. You can also reach me and connect with me on my social media or email.

Happy coding! 😄
Shivam Kaushik
Github Twitter Linkedin

no, it must be a browser issue of yours, because my navbar is flexible, it stretchs along with the size of screen, and it growths as well =)

@mekinus
Copy link
Owner Author

mekinus commented Nov 12, 2019

The navbar issue is not a problem of my page, my navbar is flexible, the previous TSEs not reported this issue to me as you can see at the first reviews. i just checked my page and it looks everything normal to me, i dont know why this review is taking too much time.

@KaushikShivam
Copy link

The navbar issue is not a problem of my page, my navbar is flexible, the previous TSEs not reported this issue to me as you can see at the first reviews. i just checked my page and it looks everything normal to me, i dont know why this review is taking too much time.

Dear @mekinus, I wasn't talking about your Navbar. It looks fine (Except that it gets a little off when you hover over the links)

I was talking about your UI. If you look at the screenshot properly, you'll realize that there's unnecessary space between the left and right of the screen whereas the design/content should span the whole width.
I went ahead and checked your project with different browsers and it's yielding the same result.

If you have any queries, please let me know.

@mekinus
Copy link
Owner Author

mekinus commented Nov 12, 2019

The navbar issue is not a problem of my page, my navbar is flexible, the previous TSEs not reported this issue to me as you can see at the first reviews. i just checked my page and it looks everything normal to me, i dont know why this review is taking too much time.

Dear @mekinus, I wasn't talking about your Navbar. It looks fine (Except that it gets a little off when you hover over the links)

I was talking about your UI. If you look at the screenshot properly, you'll realize that there's unnecessary space between the left and right of the screen whereas the design/content should span the whole width.
I went ahead and checked your project with different browsers and it's yielding the same result.

If you have any queries, please let me know.

already made the changes, and i want someone to review the code because im unable to submit the code again since is WAITING FOR REVIEW status all day long

Copy link

@Redvanisation Redvanisation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes required 🛑

Hello @mekinus

1. The website has to look like the original page to be approved, the issue you are having is explained in my comment below, please implement it.

2. After that fix the background image to the center of the screen and you are done 😃

After making the required changes please submit a new code review request

Good luck! 🍀

justify-content: space-between;
text-align: center;
border-radius: 5px;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could align the navbar to center of the screen, the issue you are having is that your navbar has an absolute position so it will be displayed in a different position depending on the screen.
first, remove the right property and replace it with the following

left: 50%;
transform: translateX(-50%);

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont know what are you talking about , you are not being clear with your request i have to guess what im supposed to do , my main image is already centered, the nav bar is flexible the nav Will not move around the page, don't try to request things that arent required to pass the review, i already made the previous changes you asked yesterday, and now you are asking me to change other elements on my page that you not stated before.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mekinus to pass the review the page should look like the original page, this is the same change I requested before, the image isn't centered and the navbar isn't in the center, the comment I made was a hint on how to fix that on all screens.

The issue here is that you used position absolute so the navbar will stay in that specific position while the other elements will change depending on the screen size.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The navbar Will shrink and stretch along with the page size, the navbar is at the center like the archive is showing, the image is at center, there is no Space to move the image , I still don't know what vou are talking about . You are asking different things each review you make , and as result slowing my progress in the Curriculum

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how the original page looks like:
Screen Shot 2019-11-12 at 7 39 11 PM

And this is how your page is actually looking:
Screen Shot 2019-11-12 at 7 40 36 PM

You are using a position absolute for the navbar it's not about the size but about the position, and this is what is causing this behavior, I gave you the code to fix that in the comment above, please fix it and submit for a re-review, I'm not asking you for things that aren't required 😃

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My page is not looking like that to me , its what im telling you , my page looks pretty normal. Like the original site

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link

@imhta imhta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Project Approved 👏

Hello @mekinus, you have done good job in this project

[Optional] The project is ok until the screen size is under 1366 width, if you rise this more, page is breaking. check that


Don't Forget to sumbit project completion form


Pon Muthu Selvam N
Feel free contact me
Github //@imhta | Twitter //@nponmuthuselvam

@mekinus mekinus merged commit fea746e into master Nov 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants