Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

feat(progressbar): allow fractional values for bar width #1761

Closed

Conversation

tjgrathwell
Copy link
Contributor

I was trying to get the animation of the progress bar to match the transition speed (.2 seconds by default) and couldn't get it to work exactly right for my code because the progress bar was always fitting to integer percentages.

I don't think there is any technical reason that the progress bar should be limited to integer steps. Here, I've used $filter('number') to truncate the progress bar value to a max precision of three decimals. Now, it can cover a larger range of values than just integers, but pathologically long fractions like 0.33333333333 won't creep in to the DOM.

@@ -5,7 +5,7 @@ angular.module('ui.bootstrap.progressbar', [])
max: 100
})

.controller('ProgressController', ['$scope', '$attrs', 'progressConfig', function($scope, $attrs, progressConfig) {
.controller('ProgressController', ['$scope', '$attrs', '$filter', 'progressConfig', function($scope, $attrs, $filter, progressConfig) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You can inject just numberFilter instead.

@bekos
Copy link
Contributor

bekos commented Feb 6, 2014

@tjgrathwell Thx for the PR. There is no technical reason to limit the progressbar to integer steps other than it's pragmatic use. What i mean is that the progressbar is just an indicator. Having a progressbar that supports 3 decimal digits, means that it's width is 100.000 pixels. I don't think that users can observe such changes or care to count pixels to know if they are at 35.567 instead of 35.568 :-) IMO, if you want to be that accurate you have to add a text hint.

Anyway, I could accept to have one decimal digit, but my main concern is the screen reader user and that have to read and be notified for such small changes. @ndarilek What is your opinion on this?

I have also left some comments on the code.

@tjgrathwell
Copy link
Contributor Author

@bekos I've made the changes you listed. I like the default behavior of numberFilter over toFixed, because numberFilter preserves "60" as "60" rather than "60.00", but this is a minor issue. Oddly numberFilter(60, 2) always returns 60.00, and numberFilter says it uses "the current locale's number formatting pattern", which I guess would sometimes be wrong. So I'm probably talking myself out of it as I type this.

I think the width value for the progressbar should have as much resolution as necessary to show down to the correct pixel on a reasonable screen, which could be 1000 pixels or more at the fringes. Maybe three decimals is too much, but I don't think the computer is going to mind. If the main issue is screen readers, we can still round the number when populating the aria attributes.

@tjgrathwell
Copy link
Contributor Author

@bekos I've pushed a revised version that uses toFixed(2) (but scrapes off unneeded zeroes) and omits decimals entirely from aria-valuenow and aria-valuetext.

@bekos
Copy link
Contributor

bekos commented Feb 7, 2014

@tjgrathwell You probably didn't notice the plus sign in front of the expression. You can do it like: +(100 * value / $scope.max).toFixed(2) to cut the trailing zeros.
Other than that I am fine with the changes.

Fractions are limited to two decimals, and are not included in aria values
@tjgrathwell
Copy link
Contributor Author

@bekos Indeed I did not notice it. Pretty clever. Sorry to throw all those 📚 words 📚 at you when that was the right answer all along.

I've pushed an updated version.

@bekos bekos closed this in 0daa7a7 Feb 8, 2014
@bekos
Copy link
Contributor

bekos commented Feb 8, 2014

Thx @tjgrathwell :-)

benjaminch pushed a commit to benjaminch/bootstrap that referenced this pull request Apr 15, 2014
Also, change the way disabled dropdownToggle is read from `attrs` instead of element property.

Closes angular-ui#1867
Closes angular-ui#1870

Add an optionnal parameter on timepicker object in order to control the visibility of the controls arrows.

chore(tests): Add unit tests for arrows control visibility

Fixing demo semi colon issue.

chore(release): Add unit tests

chore(timepicker): Add LOC to check the user's input

chore(timepicker): Remove  method for showArrowControls option

chore(timepicker): Remove Show / Hide option from demo

chore(timepicker): Hides <tr> element rather than <td> element.

chore(tests): Simplify arrows control visibility tests

chore(tests): Refactoring arrow visibility tests

refactor(rating): use `track by` in template

Closes angular-ui#1724

refactor(pagination): add `href` and `track by` in templates

Closes angular-ui#1725

fix(progressbar): allow fractional values for bar width

 * Fractions are limited to two decimals, and are not included in aria values.

Closes angular-ui#1761

fix(dropdown): use $animate for adding and removing classes

Closes angular-ui#1644

feat(datepicker): add `datepicker-mode`, `init-date` & today hint

 * Add two-way binded `datepicker-mode`.
 * Add optional `init-date` when no model value is specified.
 * Add hint for current date.
 * Use isolated scope for popup directive.
 * Use optional binding for `isOpen`.
 * Split each mode into it's own directive.

Closes angular-ui#1599

BREAKING CHANGE:
`show-weeks` is no longer a watched attribute
`*-format` attributes have been renamed to `format-*`
`min` attribute has been renamed to `min-date`
`max` attribute has been renamed to `max-date`

refactor(rating): evaluate attributes inside init function

Closes angular-ui#1590
Closes angular-ui#1768

refactor(pagination): remove unused watch

Closes angular-ui#1769

fix(typeahead): correctly higlight numeric matches

Fixes angular-ui#1777

refactor(typeahead): use `ng-if` and `track by` in template

Closes angular-ui#1722

demo(typeahead): correct demo for asyn results

Fixes angular-ui#1740

refactor(datepicker): track buttons by date instead of $index

refactor(pagination): move boundary & directions to the template

Closes angular-ui#795
Closes angular-ui#1770

feat(alert): add WAI-ARIA markup

Closes angular-ui#1806

chore(demo): fix fork button width

Closes angular-ui#1805

fix(datepicker): `Today` button should not set time

Fixes angular-ui#1726
Closes angular-ui#1808

fix(progressbar): number filter in bar template and only for percent

Closes angular-ui#1807

refactor(dropdown): remove isolated scope

Closes angular-ui#1818

feat(typeahead): add WAI-ARIA markup

Closes angular-ui#1814

refactor(accordion): transclude function in compile is deprecated.

Closes angular-ui#1789

refactor(carousel): use `track by` in template

Closes angular-ui#1723

fix(datepicker): mark input field as invalid if the date is invalid

Closes angular-ui#1845

Conflicts:
	src/datepicker/datepicker.js
	src/datepicker/test/datepicker.spec.js
benjaminch pushed a commit to benjaminch/bootstrap that referenced this pull request May 10, 2014
Also, change the way disabled dropdownToggle is read from `attrs` instead of element property.

Closes angular-ui#1867
Closes angular-ui#1870

Add an optionnal parameter on timepicker object in order to control the visibility of the controls arrows.

chore(tests): Add unit tests for arrows control visibility

Fixing demo semi colon issue.

chore(release): Add unit tests

chore(timepicker): Add LOC to check the user's input

chore(timepicker): Remove  method for showArrowControls option

chore(timepicker): Remove Show / Hide option from demo

chore(timepicker): Hides <tr> element rather than <td> element.

chore(tests): Simplify arrows control visibility tests

chore(tests): Refactoring arrow visibility tests

fix(dropdown): unbind toggle element event on scope destroy

Also, change the way disabled dropdownToggle is read from `attrs` instead of element property.

Closes angular-ui#1867
Closes angular-ui#1870

Add an optionnal parameter on timepicker object in order to control the visibility of the controls arrows.

chore(tests): Add unit tests for arrows control visibility

Fixing demo semi colon issue.

chore(release): Add unit tests

chore(timepicker): Add LOC to check the user's input

chore(timepicker): Remove  method for showArrowControls option

chore(timepicker): Remove Show / Hide option from demo

chore(timepicker): Hides <tr> element rather than <td> element.

chore(tests): Simplify arrows control visibility tests

chore(tests): Refactoring arrow visibility tests

refactor(rating): use `track by` in template

Closes angular-ui#1724

refactor(pagination): add `href` and `track by` in templates

Closes angular-ui#1725

fix(progressbar): allow fractional values for bar width

 * Fractions are limited to two decimals, and are not included in aria values.

Closes angular-ui#1761

fix(dropdown): use $animate for adding and removing classes

Closes angular-ui#1644

feat(datepicker): add `datepicker-mode`, `init-date` & today hint

 * Add two-way binded `datepicker-mode`.
 * Add optional `init-date` when no model value is specified.
 * Add hint for current date.
 * Use isolated scope for popup directive.
 * Use optional binding for `isOpen`.
 * Split each mode into it's own directive.

Closes angular-ui#1599

BREAKING CHANGE:
`show-weeks` is no longer a watched attribute
`*-format` attributes have been renamed to `format-*`
`min` attribute has been renamed to `min-date`
`max` attribute has been renamed to `max-date`

refactor(rating): evaluate attributes inside init function

Closes angular-ui#1590
Closes angular-ui#1768

refactor(pagination): remove unused watch

Closes angular-ui#1769

fix(typeahead): correctly higlight numeric matches

Fixes angular-ui#1777

refactor(typeahead): use `ng-if` and `track by` in template

Closes angular-ui#1722

demo(typeahead): correct demo for asyn results

Fixes angular-ui#1740

refactor(datepicker): track buttons by date instead of $index

refactor(pagination): move boundary & directions to the template

Closes angular-ui#795
Closes angular-ui#1770

feat(alert): add WAI-ARIA markup

Closes angular-ui#1806

chore(demo): fix fork button width

Closes angular-ui#1805

fix(datepicker): `Today` button should not set time

Fixes angular-ui#1726
Closes angular-ui#1808

fix(progressbar): number filter in bar template and only for percent

Closes angular-ui#1807

refactor(dropdown): remove isolated scope

Closes angular-ui#1818

feat(typeahead): add WAI-ARIA markup

Closes angular-ui#1814

refactor(accordion): transclude function in compile is deprecated.

Closes angular-ui#1789

refactor(carousel): use `track by` in template

Closes angular-ui#1723

fix(datepicker): mark input field as invalid if the date is invalid

Closes angular-ui#1845

Conflicts:
	src/datepicker/datepicker.js
	src/datepicker/test/datepicker.spec.js
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants