chore: don't accept string as boolean input #552
Conversation
| private _maxSize = 0; | ||
| private _page = 0; | ||
| private _pageCount = 0; | ||
| private _pageSize = 10; |
There was a problem hiding this comment.
Could we move those private fields UP, to the top of the file where they were initially?
There was a problem hiding this comment.
_pageSize should be a regular @Input.
There was a problem hiding this comment.
I agree, but the idea of this PR was to only focus on boolean inputs, and to keep numbers as is for the moment, mainly because I wasn't sure what to do with all these number inputs.
I will definitely do another PR to deal with numbers, but I wasn't sure about what to do with all the number inputs here.
We agreed that number | string should become number, but the usage of toInteger() here has another effect: it transforms decimal numbers to integers. And I am not sure if that is desired or not, or if we should just accept the user inputs as is.
Once I have an answer for that question (sorry if that wasn't clear in the issue), I will make another PR for numbers, if you don't mind.
refs ng-bootstrap#550 BREAKING CHANGE: boolean inputs in progressbar and pagination could be set by just adding the attribute (for example: `<ngb-progressbar striped></ngb-progressbar>`). This is not supported anymore. All boolean inputs must now consistently be set using the syntax `[attr]="value"` (for example: `<ngb-progressbar [striped]="true"></ngb-progressbar>`).
jnizet
force-pushed
the
chore/550-uniform-boolean-inputs
branch
from
4fbc76f
to
67c8ad5
Compare
|
Moved non-input fields to the top as before, and rebased on master. |
|
This LGTM. |
|
@jnizet just landed this one, thnx! You can kill the number part now! |
This is a first commit for #550, dealing only with boolean inputs, and not changing anything to number inputs.