Skip to content

Conversation

mmalerba
Copy link
Contributor

@mmalerba mmalerba requested a review from jelbourn January 13, 2017 20:04
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jan 13, 2017
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM, just two minor comments; add "merge ready" when you're done

}

/** Whether the slider is at its minimum value. */
get _isAtMinValue() {
Copy link
Member

Choose a reason for hiding this comment

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

I think _isMinValue works

return DISABLED_THUMB_GAP;
}
if (this._isAtMinValue && !this.thumbLabel) {
if (this._isActive) {
Copy link
Member

Choose a reason for hiding this comment

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

Could replace this inner-most if-else with a ternary

@mmalerba mmalerba added the action: merge The PR is ready for merge by the caretaker label Jan 13, 2017
@mmalerba mmalerba merged commit 737b608 into angular:master Jan 18, 2017
kara pushed a commit to kara/material2 that referenced this pull request Jan 20, 2017
* make slider disabled state match mocks

* added test

* fix lint

* min value style

* thumb label min value style

* fix lint & add tests

* fix lint

* addressed comments
@mmalerba mmalerba deleted the slider2 branch April 3, 2018 15:15
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants