Skip to content

CSS cleanup #145

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

Closed
wants to merge 4 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 68 additions & 28 deletions www/_static/swift.css
Original file line number Diff line number Diff line change
@@ -1,64 +1,104 @@
/* Common styles */
.body { color: black; background: white; margin: 0 0 0 0 }
.body {
color: black;
background: white;
margin: 0 0 0 0;

Choose a reason for hiding this comment

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

Maybe replace this with margin: 0

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

That'd be better.

}

/* No borders on image links */
a:link img, a:visited img { border-style: none }
a:link img,
a:visited img {
border-style: none;
}

address img { float: right; width: 88px; height: 31px; }
address { clear: right; }
address img {
float: right;
width: 88px;
height: 31px;
}

address {
clear: right;
}

table {
border: 2px solid black;
border-collapse: collapse;
margin-top: 1em;
margin-left: 1em;
margin-right: 1em;
margin-bottom: 1em;

Choose a reason for hiding this comment

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

Instead of setting every margin to 1em, why not just set margin: 1em?

Copy link
Author

Choose a reason for hiding this comment

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

I prefer being explicit. I prefer this notation over short hand so didn't want to change it.

Choose a reason for hiding this comment

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

Ok, but sometimes you used padding: 1em 1em 1em 1em and sometimes you split it up. I would like to see a consistency here!

Copy link
Author

Choose a reason for hiding this comment

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

Those aren't my declarations. These might be intentional choices by the author that I don't think are worth changing. The PR promotes a bunch consistency. If you want to see specific change I suggest opening up a separate PR. In my opinion there are reasons to use both shorthand and longhand and a codebase doesn't need to only have one of them as an option to be consistent. There are different contexts.

}

table { border: 2px solid black;
border-collapse: collapse; margin-top: 1em; margin-left: 1em;
margin-right: 1em; margin-bottom: 1em; }
tr, td { border: 2px solid gray; padding: 4pt 4pt 2pt 2pt; }
th { border: 2px solid gray; font-weight: bold; font-size: 105%;
background: url("http://llvm.org/img/lines.gif");
font-family: "Georgia,Palatino,Times,Roman,SanSerif";
text-align: center; vertical-align: middle; }
tr,
td {
border: 2px solid gray;
padding: 4pt 4pt 2pt 2pt;
}

th {
border: 2px solid gray;
font-weight: bold;
font-size: 105%;
background-image: url(http://llvm.org/img/lines.gif);
font-family: Georgia,Palatino,Times,Roman,sans-serif;
text-align: center;
vertical-align: middle;
}

/* Common for title and header */
h1, h2 {
color: black; background: url("http://llvm.org/img/lines.gif");
font-family: "Georgia,Palatino,Times,Roman,SanSerif"; font-weight: bold;
h1,
h2 {
color: black;
background-image: url(http://llvm.org/img/lines.gif);
font-family: Georgia,Palatino,Times,Roman,sans-serif;
font-weight: bold;
border-width: 1px;
border-style: solid none solid none;
text-align: center;
vertical-align: middle;
/* padding-left: 0pt;*/
padding-top: 1px;
padding-bottom: 2px
}


h1 {
text-align: center; font-size: 22pt;
margin: 20pt 0pt 0pt 0pt;
text-align: center;
font-size: 22pt;
margin: 20pt 0 0 0;
}

.doc_title, .title { text-align: left; font-size: 25pt }
.doc_title,
.title {
text-align: left;
font-size: 25pt;
}

h2, .doc_subsection { width: 100%;
text-align: left; font-size: 12pt;
h2,
.doc_subsection {
width: 100%;
text-align: left;
font-size: 12pt;
padding: 4pt 4pt 4pt 4pt;
margin: 1.5em 5em 0.5em 0em
}

h3 {
margin: 2.0em 0.5em 0.5em 0em;
font-weight: bold; font-style: oblique;
border-bottom: 1px solid #999999; font-size: 12pt;
margin: 2em .5em .5em 0;
font-weight: bold;
font-style: oblique;
border-bottom: 1px solid #999999;
font-size: 12pt;
width: 75%;
clear:both;
}

body p {
margin: 1em 0pt 0pt 15pt;
margin: 1em 0 0 15pt;
}


pre.grammar, pre.stdlib {
pre.grammar,
pre.stdlib {
background: #FFFFE0;
padding: 1em 0 0 0;

Choose a reason for hiding this comment

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

Can be padding: 1em 0 0

Copy link
Author

Choose a reason for hiding this comment

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

Again I prefer being explicit, didn't seem worth changing.

Choose a reason for hiding this comment

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

Who cares? I prefer press a spacebar and put a value, instead of selecting one and changing it. @philipgiuliani +1

Copy link
Author

Choose a reason for hiding this comment

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

@coobersdoobers I care. That is a code editing problem, not a css architecture problem 😉
I prefer explicitness, but changing shorthand format vs longform format for padding/margin wasn't part of the changes I wanted to make in this pull request. If you'd like to see those changes, I'd suggest opening up a new PR.

}
Expand All @@ -73,5 +113,5 @@ pre.example {
width: 33%;
background: #C0C0FF;
padding: 1em 1em 1em 1em;

Choose a reason for hiding this comment

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

padding: 1em

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 cleaner

margin: 0em 0em 0em 1em;
margin: 0 0 0 1em;
}