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

All components: Refactor to remove coerceBoolean from all inputs #1233

Closed
6 of 7 tasks
benjamincharity opened this issue Jan 17, 2019 · 0 comments
Closed
6 of 7 tasks
Assignees
Labels
💥 BREAKING CHANGE 💥 This issue is a breaking change requiring a major version bump Focus: consumer Tasks needed for the consuming repo (core) Type: chore Something that needs to be done but is not a bug or addition

Comments

@benjamincharity
Copy link
Contributor

benjamincharity commented Jan 17, 2019

Current, the library coerces all boolean inputs to a true boolean. This allows users to by 'lazy' and simply use data attributes rather than binding:

// Currently, `my-input` would be a boolean of true in the component
<my-component my-input="true"></my-component>

While this solves certain issues that arise from using a data-attribute to pass a value in that is not a string, it is working around the Angular frameworks default behavior. In the long run this is a disservice to the application engineers as our library inputs do not function exactly as default Angular inputs do.

We should revert this functionality so that our inputs function as default Angular inputs.

<!-- Currently -->
<my-component my-input="true"></my-component>

<!-- After change -->
<my-component [my-input]="true"></my-component>

💥 Breaking change 💥


  • Remove all boolean coercions from inputs
    • If the input has a setter/getter but only uses them for coercing a boolean, we should remove the set/get in favor of the simple input format (example below)
  • Verify all tests still pass
  • Verify all documentation is updated to use the correct binding ([foo]="bar" vs foo="bar")
    • JSDoc comments for each component
    • Usage docs for each component
    • Update any consumers (this should have been done during deprecation, but we should verify)
  • Write breaking change migration docs
// Simple Angular input
@Input()
myInput: myType = 'foo';
@benjamincharity benjamincharity added Focus: consumer Tasks needed for the consuming repo (core) Target: v12 Type: chore Something that needs to be done but is not a bug or addition 💥 BREAKING CHANGE 💥 This issue is a breaking change requiring a major version bump labels Jan 17, 2019
@benjamincharity benjamincharity added the Status: on hold Issues we will not address in the immediate future label Feb 4, 2019
@benjamincharity benjamincharity self-assigned this Feb 14, 2019
@benjamincharity benjamincharity removed the Status: on hold Issues we will not address in the immediate future label Apr 1, 2019
@benjamincharity benjamincharity removed their assignment Apr 1, 2019
@shani-terminus shani-terminus self-assigned this Apr 3, 2019
shani-terminus added a commit that referenced this issue Apr 4, 2019
…coerceBoolean

also cleaning up related imports

BREAKING CHANGE:
boolean values will have to be entered correctly accoring to angular in the
html([booleanvar]="true")

ISSUES CLOSED: #1233
shani-terminus added a commit that referenced this issue Apr 4, 2019
…coerceBoolean

also cleaning up related imports

BREAKING CHANGE:
boolean values will have to be entered correctly accoring to angular in the
html([booleanvar]="true")

ISSUES CLOSED: #1233
shani-terminus added a commit that referenced this issue Apr 5, 2019
BREAKING CHANGE:
text values "true" and "false" will have to be converted

ISSUES CLOSED: #1233
shani-terminus added a commit that referenced this issue Apr 8, 2019
…ated

BREAKING CHANGE:
Boolean values will have to be passed according to Angular guidelines

ISSUES CLOSED: #1233
shani-terminus added a commit that referenced this issue Apr 8, 2019
…coerceBoolean

also cleaning up related imports

BREAKING CHANGE:
boolean values will have to be entered correctly accoring to angular in the
html([booleanvar]="true")

ISSUES CLOSED: #1233
benjamincharity pushed a commit that referenced this issue Apr 8, 2019
…coerceBoolean

also cleaning up related imports

BREAKING CHANGE:
boolean values will have to be entered correctly accoring to angular in the
html([booleanvar]="true")

ISSUES CLOSED: #1233
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💥 BREAKING CHANGE 💥 This issue is a breaking change requiring a major version bump Focus: consumer Tasks needed for the consuming repo (core) Type: chore Something that needs to be done but is not a bug or addition
Projects
None yet
Development

No branches or pull requests

2 participants