Skip to content
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.

Sign up for GitHub

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

announcement css #81

Merged
merged 4 commits into from
Mar 26, 2015
Merged

announcement css #81

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fa2506f
rails generate css template
allenwq Jan 26, 2015
d492d2d
add a scss file to declare global varriables
allenwq Feb 25, 2015
371c3d3
CSS for course announcements index
allenwq Jan 26, 2015
36bf7b5
add future mixin
allenwq Feb 25, 2015
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
15 changes: 15 additions & 0 deletions app/assets/stylesheets/_variables.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//
// Variables
// --------------------------------------------------


//== Colors
//
//## Gray colors for use across Coursemology.
Copy link
Member

Choose a reason for hiding this comment

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

...where did this heading format come from? never seen it before.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm referring from bootstrap


$gray-base: #000 !default;
Copy link
Member

Choose a reason for hiding this comment

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

@wangqiang1208 @fonglh @kxmbrian @jsyeo @weiqingtoh ...american or british spelling? lol... i can't remember what the style guides say

Copy link
Member Author

Choose a reason for hiding this comment

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

Since we are using "color" instead of "colour", so american ? And bootstrap is using "gray" instead of "grey"

Copy link
Contributor

Choose a reason for hiding this comment

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

lol

Let's stick to American.

$gray-darker: lighten($gray-base, 13.5%) !default; // #222
$gray-dark: lighten($gray-base, 20%) !default; // #333
$gray: lighten($gray-base, 33.5%) !default; // #555
$gray-light: lighten($gray-base, 46.7%) !default; // #777
$gray-lighter: lighten($gray-base, 93.5%) !default; // #eee
25 changes: 25 additions & 0 deletions app/assets/stylesheets/course/announcements.css.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Place all the styles related to the course/announcements controller here.
// They will automatically be included in application.css.
// You can use Sass (SCSS) here: http://sass-lang.com/
@import "variables";
@import "mixins/future";

.course-announcements {
&.index {
.announcement {
padding-left: 8px;
Copy link
Member

Choose a reason for hiding this comment

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

I prefer units that scale, @yangshun what units do you recommend? I tend to use em for things involving block elements. px only for images...

Copy link

Choose a reason for hiding this comment

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

I think using px is ok here, as long as the layout looks fine on mobile, tablet and desktop. I'm not extremely clear about when to use relative vs absolute widths for layout sizes. But to my knowledge, Bootstrap only uses em in their font-sizes and CSS properties related to typography, such as margin of h1 elements. They use px for padding and margins for everything else, such as the grids and other UI components. Since we're using Boostrap, let's follow their convention.


.timestamp {
color: $gray-light;
}

.content {
margin-top: 10px;
}

&.future {
Copy link
Member

Choose a reason for hiding this comment

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

Should this be a mix-in?

@include future($gray-lighter);
}
}
}
}
8 changes: 8 additions & 0 deletions app/assets/stylesheets/mixins/_future.css.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
//future style
//indicates the item will be available in future

@mixin future($bg-color) {
background-color: $bg-color;
opacity: 0.9;
padding: 10px;
}