Reduce the use of `!important` for GitHub banner #744

stevenjoezhang · 2019-03-27T02:49:36Z

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines.
Tests for the changes was maked (for bug fixes / features).
- Muse | Mist have been tested.
- Pisces | Gemini have been tested.
Docs in NexT website have been added / updated (for new features).

PR Type

What kind of change does this PR introduce?

What is the current behavior?

Issue resolved: N/A

What is the new behavior?

Screenshots with this changes: N/A
Link to demo site with this changes: N/A

How to use?

In NexT _config.yml:

...

Does this PR introduce a breaking change?

Yes.
No.

@wip ready for review

Update

ivan-nginx

Non-relevant to GitHub banner.

stevenjoezhang · 2019-03-27T02:56:38Z

@ivan-nginx The title of this PR is called Reduce the use of !important

stevenjoezhang · 2019-03-27T02:57:08Z

source/css/_common/components/header/github-banner.styl

  }
  +tablet-mobile() {
    > svg {
      if (scheme == 'Pisces') || (scheme == 'Gemini') {
-        fill: #fff !important;


Here. Please, have a look.

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

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

stevenjoezhang · 2019-03-29T17:28:25Z

I believe this PR is ready to be merged now. No other work needs to be done

ivan-nginx · 2019-03-29T17:42:02Z

source/css/_schemes/Gemini/index.styl

@@ -76,8 +76,7 @@ use_seo = hexo-config('seo');
 }

 // Post delimiters.
-.post-eof,
-.post-spread {


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

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

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

ivan-nginx

All refactoring like this must be tested carefully. Screens with each scheme & resolution also needed and welcome.

ivan-nginx · 2019-03-29T17:46:36Z

source/css/_common/components/header/github-banner.styl

@@ -34,8 +43,7 @@
  +mobile() {
    > svg {
      if (scheme == 'Mist') {
-        top: inherit !important;
-        margin-top: -50px;


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

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

Of course I know what I'm doing. It looks fine in my browser.

Are you using the latest branch?

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

ivan-nginx · 2019-03-29T17:47:48Z

source/css/_common/components/post/post-eof.styl

@@ -10,7 +9,7 @@
 }

 .post:last-child {
-  .post-eof.post-eof.post-eof {


Screen? Where is this class?

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

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 %}

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.

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.

ivan-nginx

source/css/_schemes/Mist/_posts-expanded.styl (11)

1v9 · 2019-03-30T03:20:17Z

Sorry @stevenjoezhang I made test again for WIP, there's a pending status of WIP, add @wip ready for review in PR body can pass pending, look like this: WIP Successful in 1s — Ready for review

PR template need to update 😉

stevenjoezhang · 2019-03-30T06:03:57Z

Anything else need to do with this PR?

ivan-nginx

It seems all fine.

stevenjoezhang added 3 commits March 24, 2019 01:36

Merge pull request #1 from theme-next/master

83f495b

Update

Merge pull request #2 from theme-next/master

88c803b

Update

important

991570e

stevenjoezhang requested a review from ivan-nginx March 27, 2019 02:49

stevenjoezhang added CSS 🛠 Improvement labels Mar 27, 2019

stevenjoezhang mentioned this pull request Mar 27, 2019

Make CSS simpler using macro #738

Merged

15 tasks

ivan-nginx requested changes Mar 27, 2019

View reviewed changes

stevenjoezhang requested a review from ivan-nginx March 27, 2019 02:56

stevenjoezhang commented Mar 27, 2019

View reviewed changes

stevenjoezhang requested review from 1v9 and jiangtj March 28, 2019 09:36

Merge branch 'master' into important

9d12ad1

1v9 added this to the v7.1.0 milestone Mar 29, 2019

1v9 added the v6.x label Mar 29, 2019

stevenjoezhang added 2 commits March 30, 2019 01:12

margin-top: 0px

9888a55

Update index.styl

2f5ac6b

ivan-nginx reviewed Mar 29, 2019

View reviewed changes

ivan-nginx requested changes Mar 29, 2019

View reviewed changes

stevenjoezhang requested a review from ivan-nginx March 29, 2019 17:59

ivan-nginx changed the title ~~Reduce the use of !important~~ Reduce the use of !important for GitHub banner Mar 29, 2019

ivan-nginx reviewed Mar 29, 2019

View reviewed changes

ivan-nginx mentioned this pull request Mar 29, 2019

Refactor .post-eof & Siblings #759

Closed

stevenjoezhang requested a review from ivan-nginx March 30, 2019 01:57

1v9 mentioned this pull request Mar 30, 2019

Moved source/js/src to source/js #751

Merged

15 tasks

ivan-nginx approved these changes Mar 30, 2019

View reviewed changes

stevenjoezhang merged commit 56555b9 into theme-next:master Mar 30, 2019

stevenjoezhang deleted the important branch March 30, 2019 11:47

weekly-digest bot mentioned this pull request Mar 31, 2019

Weekly Digest (24 March, 2019 - 31 March, 2019) #772

Closed

tongluyang pushed a commit to tongluyang/hexo-theme-next that referenced this pull request Nov 19, 2019

Reduce the use of !important for GitHub banner (theme-next#744)

151991a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reduce the use of `!important` for GitHub banner #744

Reduce the use of `!important` for GitHub banner #744

stevenjoezhang commented Mar 27, 2019 •

edited by 1v9

Loading

ivan-nginx left a comment

stevenjoezhang commented Mar 27, 2019

stevenjoezhang Mar 27, 2019

ivan-nginx Mar 27, 2019 •

edited

Loading

stevenjoezhang Mar 27, 2019

stevenjoezhang commented Mar 29, 2019

ivan-nginx Mar 29, 2019

stevenjoezhang Mar 29, 2019

stevenjoezhang Mar 29, 2019

ivan-nginx left a comment •

edited

Loading

ivan-nginx Mar 29, 2019

stevenjoezhang Mar 29, 2019

ivan-nginx Mar 29, 2019

ivan-nginx Mar 29, 2019

stevenjoezhang Mar 29, 2019 •

edited

Loading

ivan-nginx Mar 29, 2019 •

edited

Loading

ivan-nginx Mar 29, 2019

ivan-nginx Mar 29, 2019 •

edited

Loading

ivan-nginx left a comment

1v9 commented Mar 30, 2019

stevenjoezhang commented Mar 30, 2019

ivan-nginx left a comment

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

	// 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;
	}
	}

Reduce the use of !important for GitHub banner #744

Reduce the use of !important for GitHub banner #744

Conversation

stevenjoezhang commented Mar 27, 2019 • edited by 1v9 Loading

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

How to use?

Does this PR introduce a breaking change?

ivan-nginx left a comment

Choose a reason for hiding this comment

stevenjoezhang commented Mar 27, 2019

Choose a reason for hiding this comment

ivan-nginx Mar 27, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevenjoezhang commented Mar 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivan-nginx left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevenjoezhang Mar 29, 2019 • edited Loading

Choose a reason for hiding this comment

ivan-nginx Mar 29, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivan-nginx Mar 29, 2019 • edited Loading

Choose a reason for hiding this comment

ivan-nginx left a comment

Choose a reason for hiding this comment

1v9 commented Mar 30, 2019

stevenjoezhang commented Mar 30, 2019

ivan-nginx left a comment

Choose a reason for hiding this comment

Reduce the use of `!important` for GitHub banner #744

Reduce the use of `!important` for GitHub banner #744

stevenjoezhang commented Mar 27, 2019 •

edited by 1v9

Loading

ivan-nginx Mar 27, 2019 •

edited

Loading

ivan-nginx left a comment •

edited

Loading

stevenjoezhang Mar 29, 2019 •

edited

Loading

ivan-nginx Mar 29, 2019 •

edited

Loading

ivan-nginx Mar 29, 2019 •

edited

Loading