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

feature:Implement the load bar at the top of the page #1689

Merged
merged 14 commits into from Jun 1, 2017

Conversation

shenzekun
Copy link
Contributor

see demo
use pace.js
just add two sentences

@ivan-nginx
Copy link
Collaborator

@shenzekun nice, but need to add option in config for e/d this.

@ivan-nginx
Copy link
Collaborator

@shenzekun very good. And for last, need to add option to load script internal or external (from CDN or from Server). See other same examples with this option.
P.S. Option need to cache scripts if u use own server instead github and scripts from CDN will not cached.

@shenzekun
Copy link
Contributor Author

I don' know your mean,add script to lib?

@ivan-nginx
Copy link
Collaborator

@shenzekun exactly!

@ivan-nginx
Copy link
Collaborator

@shenzekun yes, but need to add url option for path in vendors section of config. See Han example,

@ivan-nginx
Copy link
Collaborator

ivan-nginx commented May 28, 2017

@shenzekun yes, but in vendors section config file need add pace:

vendors:
...
  # Internal version: 1.0.2
  # CDN PATH?
pace:

See Han example,

_config.yml Outdated
@@ -581,6 +581,10 @@ vendors:
# https://github.com/ethantw/Han
han:

# Internal version: 1.0.2
# https://github.com/HubSpot/pace/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, and 2 direct links in comment on 1 and 2 CDN please.
For example:

# https://github.com/HubSpot/pace/
# pace: https://pace.min.js
# pace_css: https://pace-theme-flash.min.css

_config.yml Outdated
@@ -581,6 +581,10 @@ vendors:
# https://github.com/ethantw/Han
han:

# Internal version: 1.0.2
# https://github.com/HubSpot/pace/
pace: https://cdn.bootcss.com/pace/1.0.2/pace.min.js
Copy link
Collaborator

Choose a reason for hiding this comment

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

No) need this links in comments. I explain:
With the empty values (pace:) Next will use Internal versions of that scripts from lib directory.
With CDN values (pace: https://cdn.bootcss.com/pace/1.0.2/pace.min.js) Next will use that direct links path instead of Internal scripts in lib directory.
So, in default we always use Internal scripts, but if user want, he may use CDN links. But, he don't know where this links, and comment this links will be fine.

I will edit your pull now and u will see, how need to make rightly for ideal use any plugins like that. Do not edit anymore.

@ivan-nginx
Copy link
Collaborator

@shenzekun ok, now u can see how need to add scripts like that.

  1. Right script code alignment.
  2. E/D option. User may want or may not want this option.
  3. CDN option for speed of page loading. This is very important for use with own servers/VPS/VDS.

Also, if u want, u may add option with change color style of progress bar.

bars that can be modified just by changing the path
@ivan-nginx
Copy link
Collaborator

Then need to add options to change this bars or not?

@ivan-nginx ivan-nginx merged commit 83e32a8 into iissnan:master Jun 1, 2017
@ivan-nginx
Copy link
Collaborator

Thanks!

@iamxiaojianzheng
Copy link

only to change pace is invalid, i can not find reson of the question.

@my0sotis
Copy link

请问在pace设置为true之后怎么修改加载条的颜色?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants