-
Notifications
You must be signed in to change notification settings - Fork 3
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
Refactor: Use Bootstrap container, remove article & constricted #133
Conversation
✅ Deploy Preview for cal-itp-website ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
@thekaveman After this PR is merged, rebase this PR with the latest from |
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 looks mostly really good!
I think using <section>
for everything here is not semantically correct though. From the spec:
The section element is not a generic container element. When an element is needed only for styling purposes or as a convenience for scripting, authors are encouraged to use the div element instead. A general rule is that the section element is appropriate only if the element's contents would be listed explicitly in the document's outline.
The highest-level elements as <section>
e.g. <section id="deck">
, <section id="details">
seem appropriate, but within those I think <div>
might be the better choice.
Thoughts?
@thekaveman Yeah - having cc @angela-tran |
I agree we should change the inner |
closes #123
What this PR does
Removes Article
<article>
.article
CSS.<main class="container">
indefault.html
layout instead. All pages should now use<section class="row">
and<div class="col-6">
syntax logic.Removes Constricted
class="constricted"
and replaces it withrow/col/justify-center
.constricted
CSS.What this PR does NOT do
Screenshots
Removes Article refactor
Removes Constricted refactor