Skip to content
This repository was archived by the owner on Oct 26, 2020. It is now read-only.

Osagie/week1#31

Open
osagiestar wants to merge 32 commits intoCodeYourFuture:masterfrom
osagiestar:Osagie/week1
Open

Osagie/week1#31
osagiestar wants to merge 32 commits intoCodeYourFuture:masterfrom
osagiestar:Osagie/week1

Conversation

@osagiestar
Copy link
Copy Markdown

Homework week 1

@osagiestar
Copy link
Copy Markdown
Author

No issues

@osagiestar
Copy link
Copy Markdown
Author

Updating my work

Added Flukeout screenshot
@osagiestar
Copy link
Copy Markdown
Author

Attached image file

@osagiestar
Copy link
Copy Markdown
Author

Updating master repo

@osagiestar
Copy link
Copy Markdown
Author

Latest work following some merging

<div class="social">
<ul>
<li> <i class="fa fa-facebook"></i> </li>
<li> <i class="fa fa fa-twitter"></i> </li>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Applied the 'fa' class twice

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Corrected it. I have made I believe you meant only the one on facebook link. I initially tried fa fa (twice) for all of them and then fa fa fa (thrice). I got same result. Not quite sure why???

Comment thread week1/3-website/index.html Outdated
Comment thread week1/3-website/index.html Outdated
<img class="solo" src="https://i.ytimg.com/vi/4HpAujV1OOc/maxresdefault.jpg" alt="Sololearn logo">
</article>
</section>
<section class="drop">
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Similarly with these class names, if they identify a single element, they should be IDs instead

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

used IDs

Comment thread week1/3-website/index.html Outdated
<li>HTML using FreeCodeCamp</li>
<li>CSS using FreeCodeCamp</li>
</ul>
<a class="weblink" href="https://www.freecodecamp.org/osagie">This is FreeCodeCamp link</a>
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 correct use of a class! My only issue with this is that you have 2 classes, 'link' and 'weblink' which mean the same thing. Perhaps 'internal-link' and 'external-link' instead?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I have changed the link for the navigation to internal-link and the other ones on the main containers as external-link classes and styled according on the CSS file.

Comment thread week1/3-website/css/style.css Outdated
Comment thread week1/3-website/css/style.css Outdated
}

/*group class code for logos*/
.solo, .codepen, .codeorg, .capgemini {
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 the same CSS applies to 4 different classes, consider using a single class instead. See also line 112

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I have now assigned a class="course" to all of them and styled as [.course].

Comment thread week1/3-website/css/style.css Outdated
}

/*targets only the Pre-Bootcamp menu link*/
a[href*="Intro"] {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I've literally never seen this done before! That's... probably not a good sign, though. IDs work better for this kind of singling out, or child selection can also work if specified correctly.

Copy link
Copy Markdown
Author

@osagiestar osagiestar Jun 11, 2020

Choose a reason for hiding this comment

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

Corrected now. Used id selector [#active].

Comment thread week1/3-website/index.html Outdated
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants