-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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(sort): add the ability to disable sort toggling #8643
Conversation
src/lib/sort/sort-header.ts
Outdated
}, | ||
encapsulation: ViewEncapsulation.None, | ||
preserveWhitespaces: false, | ||
changeDetection: ChangeDetectionStrategy.OnPush, | ||
inputs: ['disabled: matSortHeaderDisabled'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be disabled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't go with disabled
, because while it is a component, the selector is still an attribute so it's not super obvious what you're disabling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, just a funny consequence of our rules - might loop in @jelbourn for confirmation on this but as a rule, components don't prefix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be just disabled
because the selector is dash-case (matches mat-button and mat-tab-nav-bar). It's only when the selector is camelCase that inputs should be prefixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switched it to disabled
@andrewseguin.
There's an open PR similar to this (#7786) but yours includes a way to disable the MatSort so let's go with that. |
Adds the ability for a `mat-sort-header` or `mat-sort` instance to be disabled, preventing the user from changing the sorting direction. Fixes angular#8622.
0845a60
to
0c03032
Compare
LGTM |
apply with |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Adds the ability for a
mat-sort-header
ormat-sort
instance to be disabled, preventing the user from changing the sorting direction.Fixes #8622.