Fixed formatting to resolve section indentation #7 and adjusted secti… #8
Conversation
…on height in CSS to resolve #6
|
|
||
| section { | ||
| max-height: 800px; | ||
| max-width: 800px; |
There was a problem hiding this comment.
Why do you need max-width here?
There was a problem hiding this comment.
I don't know I thought this would be better to control?
I don't need it?
There was a problem hiding this comment.
It's functional without it, but it is your choice as the designer/developer. If you want to control it the width (e.g,. if your user's screen happens to be particularly wide), then this is the right property to use.
Optional update: center the content if you're going to limit the width. Implement at your discretion.
There was a problem hiding this comment.
ok, i need hard parameters here... not that cool of a programmer yet to have discretion haha!
There was a problem hiding this comment.
I would leave the max-width off right now since there isn't really any strong evidence to suggest your users will be negatively affected by an unlimited width.
In a future release, you can set a max-width and center the content as an enhancement.
| section { | ||
| max-height: 800px; | ||
| max-width: 800px; | ||
| text-align: justify; |
There was a problem hiding this comment.
Why do you need text-align here?
There was a problem hiding this comment.
I don't know, I thought it looked better to have my sections justified?
There was a problem hiding this comment.
Okay, just asking. Your choice as the designer/developer.
There was a problem hiding this comment.
would it be better practice to give it a column? I feel like it's very microsoft word looking right now....
There was a problem hiding this comment.
To address your comment, my opinion is that it might look better to set a max-width on your content and then to center your content section. See https://getmdl.io/templates/blog/index.html for an example.
However, please open an Issue and assign it to me if you wish to discuss further to avoid scope creep (https://en.wikipedia.org/wiki/Scope_creep) on this PR.
There was a problem hiding this comment.
so each topic should be another issue? I'm confused now how to communicate....
There was a problem hiding this comment.
I guess an ideal "Issue" should contain a specific topic from which actionable items can be determined. Actionable items turn into code commits.
There was a problem hiding this comment.
gotcha. This started with a "why..?" that I didn't know the answer to. I understand now there is another way to make this look better. I guess my future commit can be centering and aligning my section text. coming up!
| max-width: 800px; | ||
| text-align: justify; | ||
| padding-top: 50px; | ||
| padding-bottom: 50px; |
There was a problem hiding this comment.
Do you need both padding-top and padding-bottom here? padding-top should be sufficient to clear the navigation bar.
There was a problem hiding this comment.
ok, got it.
For the last section, when it clicks to the target, it does not scroll all the way to the top. probs because the page has ended. how can i resolve that?
There was a problem hiding this comment.
Maybe you can add a footer element and either fill it with content or give it a sufficient height that you will achieve the desired effect.
There was a problem hiding this comment.
ok, will add a footer.
what other things do i need for this to flow better / to look real?
i want to give this a good skeleton and then make it look pretty...
There was a problem hiding this comment.
Okay to adding a footer. I think that fits within this PR.
However, regarding work to make things look pretty, please open issue and assign it to me if you wish to discuss further to avoid scope creep (https://en.wikipedia.org/wiki/Scope_creep) on this PR.
There was a problem hiding this comment.
In your index.html file, the "rule" for indentation is that child elements should be indented from their parents.
A couple examples:
<html>
<head>
</head>
<body>
</body>
</html>
<body>
<nav>
<ul>
<li><a href="#home">Home</a></li>
...
</ul>
</nav>
<section>
<h4>...</h4>
</section>
...
</body>
…on height in CSS to resolve #6