-
Notifications
You must be signed in to change notification settings - Fork 27
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
Change: semantic HTML-elements for the pages main sections and corresponding CSS-rules #84
Conversation
Is there anyone? Everybody in the Easter holidays? ;-) I hoped to get a reaction to satisfy myself to be on the right way. Same for #85. |
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.
Sorry for the delay in getting to this
Just the one minor nitpick. I feel like I'd prefer it if the "unnecessary" ids were removed as well - what did you mean by "I can't ensure, that they are not addressed from elsewhere" ? Seems like something a text search can deal with :)
margin-bottom: 20px; | ||
text-align: center; |
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.
I feel like this change might be confusing, given the name of the class. The class is only used in 2 places - rename?
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.
Seems to be very plausible to change the class name. :-)
I'll search the sources. If I'll find something I can't comprehend I'll ask here. Otherwise I'll add a commit to remove the ids. |
Ping @LordAro : three tickets with near identical suggested changes, but no activity in a long time. What do you want to do with the pull requests? |
After pushing the task more than a half year along, I came into trouble because of the many changes in the meantime. Because of merging of these changes into my branch it came from three to around 20 commits. I tried to squash this into one commit but that would reintroduce all your changes a second time. So I decided to close this PR and opened a new one (#113) with only my own changes (cherry-picked and squashed). |
Changed HTML
div#header
intoheader#header
div#navigation
intonav#navigation
div#content-main
intomain#content-main
div#content-bottom
intofooter#content-bottom
and separated it frommain
Changed CSS
body
and replaced it by a background colour#header
intobody > header
(combination because a page can contain several elementsheader
(i.e. furtherheader
for blog articles))#navigation
intobody > nav
(combination because a page can contain more than one elementsnav
)#content-main
intomain
("There can be only one")float
becausefloat
extracts the element from the page element flow#content-bottom
intobody > footer
(combination because a page can contain several elementsfooter
(i.e. furtherfooter
for blog articles))main