-
-
Notifications
You must be signed in to change notification settings - Fork 12
Have added more details in HTML #2
base: master
Are you sure you want to change the base?
Conversation
index.html
Outdated
| <header> | ||
| <h1 class="heading">CYF</h1> | ||
| </header> | ||
| <nav-bar> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nav-bar is not a valid HTML element .. it should just be nav
index.html
Outdated
| <article> | ||
| <h3>What We Do</h3> | ||
| <p> | ||
| <strong class="biggertext">CYF</strong> is a fictitious company-lorem ipsum dolor sit amet, whatever. CYF |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
class names, either use dashes as separators which is preferred (so bigger-text) or camelCase. The important thing is to be consistent, and not have it all lowercase as it is hard to read
| @@ -0,0 +1,97 @@ | |||
| .heading { | |||
| font-family: 'Apple-Chancery', cursive; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this font will probably only work on your machine - you can import Google fonts to ensure that the font is loaded for all users https://fonts.google.com/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for all your help. I am not very sure how to put all google fonts.. Please help me.. Thanks..
main.css
Outdated
| .heading { | ||
| font-family: 'Apple-Chancery', cursive; | ||
| font-style: italic; | ||
| font-size: 60px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try using "em" and "rem" instead of "px" for units. Read about them if you don't know what they are - they help with doing responsive design and make it scale easier
main.css
Outdated
| .bordercontainer { | ||
| /*border: 3px solid blue;*/ | ||
| /*display: flex;*/ | ||
| margin-right: 29%; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is not typical to use percentages for margins and paddings, it works but it is not very standard. You can probably achieve the same using em.
| var request = new XMLHttpRequest(); | ||
| request.open('GET',url) | ||
|
|
||
| request.onload = function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very nice usage of onload (make sure the code is indented properly though)
main.js
Outdated
| } | ||
|
|
||
|
|
||
| var SHnews = document.querySelector('#news-btn'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again bad variable names
| } | ||
|
|
||
| function ShowHide(element) { | ||
| if (element.className === "hidden") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice and simple implementation. well done.
main.js
Outdated
| var numberInputWV = numberInput.value; | ||
|
|
||
| var SubmitFormIsValid = true; | ||
| if (nameInputWV.length === 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this check can be if (nameInput.length)
(also pay attention to variable names - what does WV stand for?)
| } | ||
|
|
||
| if (emailInputWV.length === 0 || emailInputWV.indexOf('@') === -1) { | ||
| emailInput.className = 'emailInput inValid'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is correct for the requirement .. Now look into Regular Expressions. See how can you find a RegExp that validates emails. Regular expressions can be quite frightening but at least know how to use them, and understand what they're used for.
|
Hello, I have modified the page again. Please have a look and give me feedback if you have time. Thank you so much for your help. |
Hello,
I have just added more details in HTML but haven't started css yet.