-
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
refactor(expansion): avoid unnecessary spacing check #17693
refactor(expansion): avoid unnecessary spacing check #17693
Conversation
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.
LGTM
@@ -185,7 +185,7 @@ export class MatExpansionPanel extends CdkAccordionItem implements AfterContentI | |||
// We don't need to subscribe to the `stateChanges` of the parent accordion because each time | |||
// the [displayMode] input changes, the change detection will also cover the host bindings | |||
// of this expansion panel. | |||
return (this.expanded ? this.accordion.displayMode : this._getExpandedState()) === 'default'; | |||
return this.expanded && this.accordion.displayMode === 'default'; |
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'm not sure the comment above it makes sense anymore either.
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, I'll remove it since it doesn't seem helpful in that place. I got confused too.
The check where `expanded` equals to `false`, and where the value of the `_getExpandedState` method is compared to `default` will always evaluate to `false`. This is because the method return value does not have any overlap with `default`. Hence the check is unnecessary. This seems to be a leftover from angular@f9bd5d4.
02206cf
to
b311f93
Compare
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.
LGTM
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. |
The where expanded equals to
false
, and where the return value ofthe
_getExpandedState
method is compared todefault
will alwaysevaluate to
false
. This is because the method return value does nothave any overlap with
default
. It only returnsexpanded
orcollapsed
.This seems to be a leftover from f9bd5d4.