fix(progressbar): display progressbar correctly for invalid max value

[chrstnbrn]

closes #3386

Before submitting a pull request, please make sure you have at least performed the following:

• read and followed the CONTRIBUTING.md guide.
• built and tested the changes locally.
• added/updated any applicable tests.
• added/updated any applicable API documentation.
• added/updated any applicable demos.

Currently the following is displayed when max is set to 0, null or undefined:

After this change the max value will be set to 100 if it is invalid so that a progressbar with a valid value can be displayed.

[codecov-io]

Codecov Report

Merging #3400 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master   #3400      +/-   ##
=========================================
+ Coverage    91.1%   91.1%   +<.01%     
=========================================
  Files          95      95              
  Lines        2787    2789       +2     
  Branches      519     520       +1     
=========================================
+ Hits         2539    2541       +2     
  Misses        189     189              
  Partials       59      59

Flag	Coverage Δ
#e2e	55.5% <33.33%> (-0.04%)	⬇️
#unit	88.23% <100%> (ø)	⬆️

Impacted Files	Coverage Δ
src/progressbar/progressbar.ts	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5610abe...7ccc031. Read the comment docs.

[peterblazejewicz]

@chrstnbrn
I believe this guard should also catch negative values and negative infinite value?
This is how the native progress handless max value (it requires positive, non-0 and finite values)
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/progress
just for consideration

[chrstnbrn]

@peterblazejewicz I changed the code to handle negative values. I also updates the docs to say that max should be a positive number.

[maxokorokov]

Hey, @chrstnbrn!

Thanks for updating the docs and adding tests.

However, I don't think the implemented behaviour is correct though → you should adjust the max value, not the percentage value.

Bootstrap defaults to 100 is max is not specified, so let's just adjust the max when setting it.
Actually the standard <progress> element does the same thing, but they default to 1, not 100 (https://developer.mozilla.org/en-US/docs/Web/HTML/Element/progress)

All your tests should return expect(progressCmp.getPercentValue()).toBe(25);
Having value=25 max=0 to show 100 is very confusing.

So, I'd suggest adjusting the max value in the setter like so:

  private _max: number;

  @Input() set max(max: number) {
    this._max = !isNumber(max) || max <= 0 ? 100 : max;
  }

  get max(): number {
    return this._max;
  }

And document the behaviour a little better (see comment in code).

• adjust max value when setting it
• make tests display 25
• update documentation

[chrstnbrn]

Hi @maxokorokov,

thank you for your feedback! I agree that adjusting the invalid max value makes more sense and made the changes you suggested.

[maxokorokov]

Thanks a lot, @chrstnbrn, LGTM!