Skip to content

Reduce the use of !important for GitHub banner #744

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

Merged
stevenjoezhang merged 6 commits into from
Mar 30, 2019
Merged

Reduce the use of !important for GitHub banner #744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
83f495b
Merge pull request #1 from theme-next/master
stevenjoezhang Mar 23, 2019
88c803b
Merge pull request #2 from theme-next/master
stevenjoezhang Mar 26, 2019
991570e
important
stevenjoezhang Mar 27, 2019
9d12ad1
Merge branch 'master' into important
stevenjoezhang Mar 28, 2019
9888a55
margin-top: 0px
stevenjoezhang Mar 29, 2019
2f5ac6b
Update index.styl
stevenjoezhang Mar 29, 2019
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
4 changes: 1 addition & 3 deletions layout/_partials/github-banner.swig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
{% if theme.github_banner.enable %}
{% set github_banner_bg_color = theme.android_chrome_color %}
{% set github_banner_fg_color = '#fff' %}
{% set github_URL = theme.github_banner.permalink %}
{% set github_title = theme.github_banner.title %}

{% set github_image = '<svg width="80" height="80" viewBox="0 0 250 250" style="fill: ' + github_banner_bg_color + '; color: ' + github_banner_fg_color + '; position: absolute; top: 0; border: 0; right: 0;" aria-hidden="true"><path d="M0,0 L115,115 L130,115 L142,142 L250,250 L250,0 Z"></path><path d="M128.3,109.0 C113.8,99.7 119.0,89.6 119.0,89.6 C122.0,82.7 120.5,78.6 120.5,78.6 C119.2,72.0 123.4,76.3 123.4,76.3 C127.3,80.9 125.5,87.3 125.5,87.3 C122.9,97.6 130.6,101.9 134.4,103.2" fill="currentColor" style="transform-origin: 130px 106px;" class="octo-arm"></path><path d="M115.0,115.0 C114.9,115.1 118.7,116.5 119.8,115.4 L133.7,101.6 C136.9,99.2 139.9,98.4 142.2,98.6 C133.8,88.0 127.5,74.4 143.8,58.0 C148.5,53.4 154.0,51.2 159.7,51.0 C160.3,49.4 163.2,43.6 171.4,40.1 C171.4,40.1 176.1,42.5 178.8,56.2 C183.1,58.6 187.2,61.8 190.9,65.4 C194.5,69.0 197.7,73.2 200.1,77.6 C213.8,80.2 216.3,84.9 216.3,84.9 C212.7,93.1 206.9,96.0 205.4,96.6 C205.1,102.4 203.0,107.8 198.3,112.5 C181.9,128.9 168.3,122.5 157.7,114.1 C157.9,116.9 156.7,120.9 152.7,124.9 L141.0,136.5 C139.8,137.7 141.6,141.9 141.8,141.8 Z" fill="currentColor" class="octo-body"></path></svg>' %}
{% set github_image = '<svg width="80" height="80" viewBox="0 0 250 250" aria-hidden="true"><path d="M0,0 L115,115 L130,115 L142,142 L250,250 L250,0 Z"></path><path d="M128.3,109.0 C113.8,99.7 119.0,89.6 119.0,89.6 C122.0,82.7 120.5,78.6 120.5,78.6 C119.2,72.0 123.4,76.3 123.4,76.3 C127.3,80.9 125.5,87.3 125.5,87.3 C122.9,97.6 130.6,101.9 134.4,103.2" fill="currentColor" style="transform-origin: 130px 106px;" class="octo-arm"></path><path d="M115.0,115.0 C114.9,115.1 118.7,116.5 119.8,115.4 L133.7,101.6 C136.9,99.2 139.9,98.4 142.2,98.6 C133.8,88.0 127.5,74.4 143.8,58.0 C148.5,53.4 154.0,51.2 159.7,51.0 C160.3,49.4 163.2,43.6 171.4,40.1 C171.4,40.1 176.1,42.5 178.8,56.2 C183.1,58.6 187.2,61.8 190.9,65.4 C194.5,69.0 197.7,73.2 200.1,77.6 C213.8,80.2 216.3,84.9 216.3,84.9 C212.7,93.1 206.9,96.0 205.4,96.6 C205.1,102.4 203.0,107.8 198.3,112.5 C181.9,128.9 168.3,122.5 157.7,114.1 C157.9,116.9 156.7,120.9 152.7,124.9 L141.0,136.5 C139.8,137.7 141.6,141.9 141.8,141.8 Z" fill="currentColor" class="octo-body"></path></svg>' %}

{{ next_url(github_URL, github_image, {class: 'github-corner', title: github_title, "aria-label": github_title}) }}
{% endif %}
36 changes: 22 additions & 14 deletions source/css/_common/components/header/github-banner.styl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
@keyframes octocat-wave {
0%, 100% {
transform: rotate(0);
}
20%, 60% {
transform: rotate(-25deg);
}
40%, 80% {
transform: rotate(10deg);
}
}

.github-corner {
scheme = hexo-config('scheme');
bg_color = unquote(hexo-config('android_chrome_color'));
Expand All @@ -6,22 +18,19 @@
:hover .octo-arm {
animation: octocat-wave 560ms ease-in-out;
}
@keyframes octocat-wave {
0%, 100% {
transform: rotate(0);
}
20%, 60% {
transform: rotate(-25deg);
}
40%, 80% {
transform: rotate(10deg);
}
> svg {
fill: bg_color;
color: #fff;
position: absolute;
top: 0;
border: 0;
right: 0;
}
+tablet-mobile() {
> svg {
if (scheme == 'Pisces') || (scheme == 'Gemini') {
fill: #fff !important;
Copy link
Contributor Author

@stevenjoezhang stevenjoezhang Mar 27, 2019

Choose a reason for hiding this comment

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

Here. Please, have a look.

Copy link
Member

@ivan-nginx ivan-nginx Mar 27, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

U say u fix hide() with !important. GitHub banner nothing to do with this.
Also, u brake foreground styles for GitHub banner.

Copy link
Contributor Author

@stevenjoezhang stevenjoezhang Mar 27, 2019

Choose a reason for hiding this comment

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

My mistake, it's not just about hide()
And I have tested, it's fine in all resolution, doesn't brake foreground styles

color: bg_color !important;
fill: #fff;
color: bg_color;
}
}
.github-corner:hover .octo-arm {
Expand All @@ -34,8 +43,7 @@
+mobile() {
> svg {
if (scheme == 'Mist') {
top: inherit !important;
margin-top: -50px;
Copy link
Member

@ivan-nginx ivan-nginx Mar 29, 2019

Choose a reason for hiding this comment

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

I believe u not test on all schemes with all resolutions:

image

Don't need remove parameters if u don't know for what they are. If parameter present, then he's needed for something.

Copy link
Contributor Author

@stevenjoezhang stevenjoezhang Mar 29, 2019

Choose a reason for hiding this comment

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

Of course I know what I'm doing. It looks fine in my browser.
屏幕快照 2019-03-30 上午1 56 17
Are you using the latest branch?

Copy link
Member

@ivan-nginx ivan-nginx Mar 29, 2019

Choose a reason for hiding this comment

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

Oh, my fault: i forgot to checkout your pull.

top: inherit;
if mobile_layout_economy {
+mobile-small() {
margin-top: initial;
Expand Down
3 changes: 1 addition & 2 deletions source/css/_common/components/post/post-eof.styl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
.posts-expand {
.post-eof {
show();
margin: $post-eof-margin-top auto $post-eof-margin-bottom;
width: 8%;
height: 1px;
Expand All @@ -10,7 +9,7 @@
}

.post:last-child {
.post-eof.post-eof.post-eof {
Copy link
Member

@ivan-nginx ivan-nginx Mar 29, 2019

Choose a reason for hiding this comment

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

Screen? Where is this class?

Copy link
Contributor Author

@stevenjoezhang stevenjoezhang Mar 29, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure. But it's same as .post-eof, no need to repeat 3 times

Copy link
Member

@ivan-nginx ivan-nginx Mar 29, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

This thing need to add delimeter between posts in homepage. And of course, last delimeter no needed, that's why display none on last child:

hexo-theme-next/layout/_macro/post.swig

Lines 449 to 452 in 022b9cc

{% set isLast = loop.index % page.per_page === 0 %}
{% if is_index and not isLast %}
<div class="post-eof"></div>
{% endif %}

Copy link
Member

@ivan-nginx ivan-nginx Mar 29, 2019

Choose a reason for hiding this comment

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

But see, in Gemini I make same feature with another way:

hexo-theme-next/source/css/_schemes/Gemini/index.styl

Lines 45 to 53 in 022b9cc

// When blocks are siblings (homepage).
#posts > article + article {
.post-block {
margin-top: $sidebar-offset;
// Rewrite shadows & borders because all blocks have offsets.
box-shadow: $box-shadow;
border-radius: $border-radius;
}
}

IDK which method is better, but I think need to choose only one.

Copy link
Member

@ivan-nginx ivan-nginx Mar 29, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

So, in Muse and Pisces – .post-eof enabled.
In Mist – disabled and in Gemini – disabled & implemented with another way.

Need to do refactor for that and join 2 spitted functions into better one.

.post-eof {
hide();
}
}
3 changes: 1 addition & 2 deletions source/css/_schemes/Gemini/index.styl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ use_seo = hexo-config('seo');
}

// Post delimiters.
.post-eof,
.post-spread {
Copy link
Member

@ivan-nginx ivan-nginx Mar 29, 2019

Choose a reason for hiding this comment

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

Are we remove this class? I don't remember...

Copy link
Contributor Author

@stevenjoezhang stevenjoezhang Mar 29, 2019

Choose a reason for hiding this comment

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

I didn't find post-spread in NexT... seems it has been removed

Copy link
Contributor Author

@stevenjoezhang stevenjoezhang Mar 29, 2019

Choose a reason for hiding this comment

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

Here: https://github.com/theme-next/hexo-theme-next/pull/357/files. .post-spread no longer exist

.post-eof {
hide();
}

Expand Down