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

Support for custom tokens in format #15

Merged
merged 5 commits into from
Sep 14, 2017
Merged

Conversation

tobiasps
Copy link
Contributor

This pull request provides the ability to render custom data to the progress bar.

In options you define extra tokens and their default value. When updating you can then optionally set the values for the tokens.

I'm using this to be able to show transfer speed in my progress info. Let me know if you want any changes.

@AndiDittrich AndiDittrich added this to the v1.6 milestone Sep 12, 2017
@AndiDittrich
Copy link
Member

Hi Tobias,

thanks for your contribution! adding custom values/placeholders is a great enhancement :)
before merging, the following changes should be applied:

  • Variable Naming: this.custom is a "bad" name for variables - i would prefer to change it to this.payload as this is a more convinient naming scheme for such kind of additional data
  • Example: please do not alter the basic example. add a new example function called Example5 :)

best regards, Andi

@tobiasps
Copy link
Contributor Author

Hi @AndiDittrich, please review my PR now. I have renamed to payload, reverted Example 1 and added Example 5.

@AndiDittrich AndiDittrich merged commit d3567e3 into npkgz:master Sep 14, 2017
@AndiDittrich
Copy link
Member

Hi @tobiasps,

i've changed the initialization of the payload data in 83c7e73

  • Within the constructor, this.payload is initialized as empty object
  • Within the start() method, the initial payload data can be passed as 3rd argument

These changes allows users to re-use the bar with different intial values as desired.

br, Andi

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

Successfully merging this pull request may close these issues.

2 participants