Skip to content

[MDL2]: Checkbox & ChoiceGroup - #45

Merged
Aniket Handa (atneik) merged 10 commits into
beta/mdl2from
anihan/mdl2/checkbox-choicegroup
Aug 2, 2016
Merged

[MDL2]: Checkbox & ChoiceGroup #45
Aniket Handa (atneik) merged 10 commits into
beta/mdl2from
anihan/mdl2/checkbox-choicegroup

Conversation

@atneik

Copy link
Copy Markdown
Contributor
  • MDL2 updates for Checkbox & ChoiceGroup
  • RTL

screen shot 2016-08-01 at 5 58 49 pm

screen shot 2016-08-01 at 5 58 16 pm

@msftclas

Hi Aniket Handa (@atneik), I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!


It looks like you're a Microsoft contributor (Aniket Handa). If you're full-time, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla.microsoft.com.

TTYL, MSBOT;

Comment thread src/components/Checkbox/Checkbox.tsx Outdated
className='ms-ChoiceField-input'
className='ms-Checkbox-input'
type='checkbox'
role='checkbox'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Role="checkbox" isn't needed since it's taken from the type.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removing

// Checkbox styles

$ms-checkbox-field-size: 20px;
$ms-checkbox-transition-duration: 200ms;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not in the scope of this PR to change now, but boy, it would be nice if these animation values could be reconciled against what's in the animation library (or rather, have the animation library updated so we don't have one-off values like throughout the components).

Comment thread src/components/Checkbox/Checkbox.tsx Outdated
<label
htmlFor={ this._id }
className={ css('ms-Checkbox-field', isChecked && 'is-checked', !isEnabled && 'is-disabled') }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: newline should be removed

@Jahnp

Peter Jahn (Jahnp) commented Aug 2, 2016

Copy link
Copy Markdown
Member

Approved 🍷

This is awesome Aniket Handa (@atneik)! Really great work. Had a few small suggestions, but otherwise this looks good!

Approved with PullApprove

@atneik
Aniket Handa (atneik) merged commit 8c6bfe7 into beta/mdl2 Aug 2, 2016
@atneik
Aniket Handa (atneik) deleted the anihan/mdl2/checkbox-choicegroup branch August 2, 2016 22:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants