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.

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

Fix PR screen in github #17

Merged
merged 7 commits into from
Oct 10, 2017
Merged
Changes from 4 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
50 changes: 47 additions & 3 deletions sites/github.styl
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@

// ** Misc

.link-gray-dark
a.link-gray-dark
Copy link
Owner

Choose a reason for hiding this comment

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

Since you added the plain class selector, can we remove the a.link-gray-dark selector?

color()
.text-gray-dark
color()

Expand Down Expand Up @@ -251,6 +251,9 @@ a.filter-item
> h3
background-color color-background-highlight-extra-less
color emphasized
.more-repos
background-color color-background-highlight-extra-less
box-shadow inset 0 1px 0 color-background-highlight-extra
Copy link
Owner

Choose a reason for hiding this comment

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

Do you need to add i at the end to make it !important?


.boxed-group-inner
background-color()
Expand All @@ -269,14 +272,37 @@ a.filter-item
.branch-action-state-dirty
.branch-action-state-unstable
.branch-action-state-unknown
.branch-action-icon
.branch-action-icon
Copy link
Owner

Choose a reason for hiding this comment

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

Why did you remove the indentation here? IIUC it will now make all .branch-action-icon icons yellow.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, this is a mistake, I didn't know what the difference was, correcting!

Copy link
Owner

Choose a reason for hiding this comment

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

No prob. Indentation with selectors is equivalent to putting them on the same line separated by a space in regular CSS, so this selector only applies to .branch-action-icon classed items that are children of the -state-* selectors above it.

background-color yellow i
color white i

.text-shadow-light
.branch-group-name
.branch-summary
color color-text i
Copy link
Owner

Choose a reason for hiding this comment

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

The color mixin applies color-text i automatically, so you can change this to just color.

text-shadow none i

.branch-a-b-count .meter
background-color yellow

.more-branches
.branch-group-heading
.branch-infobar
.branch-name
border-color comment
background-color highlight

.State
.State:visited
.octoicon-git-pull-request
color color-background-highlight i
Copy link
Owner

@alphapapa alphapapa Oct 10, 2017

Choose a reason for hiding this comment

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

The color mixin applies i automatically, so it can be left off here, and in the other places that call color.

.State--red
background-color red
color color-background i
.State--green
background-color green
color color-background i

// ** build
.build-status-item
background-color color-background-highlight-extra-less
Expand Down Expand Up @@ -583,10 +609,13 @@ a.label
color transparentify(green, color-background, 0.65)

.octicon-alert
.octicon-check
color white
.octicon-x
color red
.octicon-check
color green
Copy link
Owner

Choose a reason for hiding this comment

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

Without having investigated it myself, is this the best way to do this? I'm guessing that I set .octicon-check to white for when it appears on a green background, IIRC. Do we need to use a more specific selector for when it should be green?

OTOH maybe the CSS has changed and this is now the right way to do it. :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a bit finiky. Github uses the .text-green selector to make the checks green. I tried overriding it, but it seemed the .octicon general selector was still overriding it. .octicon-check was the most specific selector I could find which worked.

I don't think that the check is ever on a green background, I've only seen it on the background, like in branches.

If there was a way of 'turning off' the .octicon custom style just for a class, then we could do it this way (but I don't know if that's possible).

Copy link
Owner

Choose a reason for hiding this comment

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

Ok, sounds good, thanks!

.octicon-primitive-dot
color yellow

.octicon-clippy
color emphasized
Expand Down Expand Up @@ -649,11 +678,15 @@ a.label
.merge-status-item
background-color color-background-highlight-extra-less

.merge-message
background-color color-background-highlight-extra-less-less

.pr-toolbar
background-color()

// *** Code reviews


.review-thread-reply
background-color highlight

Expand Down Expand Up @@ -681,6 +714,17 @@ a.label
img
background-color()

.range-editor
background-color highlight

.compare-pr-placeholder
background-color color-background-highlight-extra-less-less

ul.comparison-list > li.title
background-color color-background-highlight-extra-less
ul.comparison-list
background-color color-background-highlight-extra-less-less

// ** repository
.repository-description
color()
Expand Down