Skip to content
This repository was archived by the owner on Jan 14, 2024. It is now read-only.

kossivi/Responsive-Cake-Webpage - #76

Closed
Kos0606 wants to merge 7 commits into
CodeYourFuture:masterfrom
Kos0606:master
Closed

kossivi/Responsive-Cake-Webpage#76
Kos0606 wants to merge 7 commits into
CodeYourFuture:masterfrom
Kos0606:master

Conversation

@Kos0606

@Kos0606 Kos0606 commented Nov 27, 2020

Copy link
Copy Markdown

Your Details

  • Your Name:
  • Your City:
  • Your Slack Name:

Homework Details

  • Module:
  • Week:

Comment thread index.html

</div>

<br>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

While it's valid HTML, generally it's better to use css padding and margin instead of
to create spacing.

@pddys pddys left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This exercise looks unfinished. The instructions were to start from a mobile width and then use media queries to create 3 different versions of the layout. I can only see one layout at desktop widths.

Was there more to this that was not committed? Otherwise this looks like the starting point, rather than a finished assignment.

Comment thread index.html
<br>
<br>

<div class="nav-bar">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This could have been an unordered list? ul? or a nav?

As these aren't really divs, it's better to be semantic in your HTML

Comment thread index.html
<div>IMAGE CAROUSEL
<br>
<br>
<p>IMG1</p>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If these are images, then it'd be better to use the image tag and some placeholder images.

Comment thread index.html
<br>
<br>

<footer class="extra-info">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is a good use of a semantic HTML tag

Comment thread index.js
@@ -0,0 +1,10 @@
function participant(students, mentors) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Was this from a different task?

Comment thread style.css

.flex-container > div {
background-color: #a9a9a9;
margin: 0.627em;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There are some values here without much context as to why they exist. These are called 'magic numbers'.

It may be useful to add a comment explaining why you used these decimals.

Comment thread style.css
}

.nav-bar {
display: flex;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good use of flex.

Comment thread style.css
margin: 0.627em;
padding: 0.1em;
font-size: 1.882em;
color: white;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You've mixed hex colors and web safe colours here. It's best to stick to one format.

Comment thread style.css
font-size: 1.875em;
}

.main-content div:nth-child(2) > p {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In this case, it may have been better to add a new class, than target paragraphs using nth-child.

Comment thread style.css
}

@media only screen and (max-width: 100%) {
body {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Was this left unfinished?

Comment thread style.css
font-size: 1.875;
}

@media only screen and (max-width: 100%) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

max-width would take in absolute values rather than 100%.

@github-actions

Copy link
Copy Markdown

Your coursework submission has been closed because nobody has interacted with it in six weeks. You are welcome to re-open it to get more feedback.

@github-actions github-actions Bot added the Stale label May 14, 2021
@github-actions github-actions Bot closed this May 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants