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

feat(business/checkbox): expose checkbox component for business #51

Merged
merged 6 commits into from
Jun 28, 2019

Conversation

efux
Copy link
Contributor

@efux efux commented Jun 24, 2019

Exposes the checkbox component for business applications.

The business library also offers a styling for indeterminated state.

Styling should be according to the digital guidelines of SBB for business applications.

@@ -16,6 +16,7 @@

<div class="sbb-checkbox-container">
<sbb-icon-tick class="sbb-checkbox-checked"></sbb-icon-tick>
<div *ngIf="!checked" class="sbb-checkbox-indeterminate"></div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This I would like to avoid.
Indeterminate does not exist in public.
Is this maybe possible with pure css? (:content)
However, how can the consumer activate the indeterminate state?
I think extending the component might be the better approach here.

Copy link
Contributor Author

@efux efux Jun 25, 2019

Choose a reason for hiding this comment

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

I created a distinct checkbox component for business, which has an indeterminate state. The checkbox can now be visually unchecked (and therefore not indeterminated), indeterminate (indeterminate and unchecked or indeterminate and checked), checked (and therefore not indeterminated).

Please note, that I don't update the indeterminate state of the native element, however, I update ariaChecked. Do you think this is sufficient or should I update the native element as well?

The indeterminate "feature" will be useful, when nesting checkboxes, which will be a requirement for business really soon.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please create a new template for the business variant, which forwards the indeterminate state to the native checkbox element (<input ... [indeterminate]="indeterminate" ...>).
indeterminate can probably be reduced to a simple property, instead of getter/setter.
You can also attach @HostBinding('class.sbb-checkbox-indeterminate') to the indeterminate property. indeterminateClass is unnecessary.

@efux efux requested a review from kyubisation June 25, 2019 12:01
Copy link
Collaborator

@kyubisation kyubisation left a comment

Choose a reason for hiding this comment

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

LGTM. Danke für die Anpassungen. 😃

@kyubisation kyubisation merged commit d666901 into master Jun 28, 2019
@kyubisation kyubisation deleted the feature/business-checkbox branch June 28, 2019 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants