feat(md-checkbox): Implement md-checkbox. #131
Conversation
googlebot
added
the
cla: yes
traviskaufman
force-pushed
the
feat-md-checkbox
branch
from
3246152
to
5901ed1
Compare
| xmlns="http://www.w3.org/2000/svg" | ||
| xmlns:xlink="http://www.w3.org/1999/xlink" | ||
| viewBox="0 0 24 24" | ||
| xml:space="preserve"> |
There was a problem hiding this comment.
Are xml:space and xmlns:xlink necessary?
There was a problem hiding this comment.
I don't think xmlns:xlink is but I'd argue xml:space is because it explicitly instructs browsers how to parse the SVG XML. From the MDN Docs
Note that this attribute influences the way a browser should parse the xml content and therefore will change the way the DOM is built.
traviskaufman
force-pushed
the
feat-md-checkbox
branch
from
dab24e3
to
0ea6602
Compare
| @@ -18,6 +18,11 @@ $pi: 3.14159265; | |||
| // Easing Curves | |||
| // TODO(jelbourn): all of these need to be revisited | |||
|
|
|||
| // The default animation curves used by material design. | |||
| $linear-out-slow-in-timing-function: cubic-bezier(0.0, 0.0, 0.2, 0.1) !default; | |||
There was a problem hiding this comment.
Can you prefix these variables with md-? (it's a bug that the existing ones don't have a prefix)
traviskaufman
force-pushed
the
feat-md-checkbox
branch
2 times, most recently
from
3589e32
to
e579751
Compare
|
@jelbourn all changes made! |
| var fixture: ComponentFixture; | ||
|
|
||
| beforeEach(function(done: () => void) { | ||
| builder.createAsync(CheckboxEndAlignedController).then(function(_fixture_) { |
There was a problem hiding this comment.
Rather than _fixture_, I'd be fine with just saying f (since the first thing you do is set it to fixture)
traviskaufman
force-pushed
the
feat-md-checkbox
branch
from
e579751
to
1d20875
Compare
|
@jelbourn all changes made! |
|
LGTM @kara can you do a pass? I think it's generally good to have a second set of eyes on any larger change. |
|
@traviskaufman Looks like CI is failing: Is this something we can change in the code? Looks like there's a missing polyfill. |
| } | ||
|
|
||
| .md-checkbox-background { | ||
| @extend %md-checkbox-background; |
There was a problem hiding this comment.
Nitpick: do you think it's confusing that the class and the placeholder share the same name? Since the comment above cites the "outer box", that might be a more descriptive name for the placeholder.
traviskaufman
force-pushed
the
feat-md-checkbox
branch
from
1d20875
to
e5a1cde
Compare
|
@jelbourn @kara @marcysutton all changes made! |
|
@traviskaufman looks like there a few failures left on CI? |
|
@jelbourn looking into it now |
traviskaufman
force-pushed
the
feat-md-checkbox
branch
3 times, most recently
from
f1c1693
to
c9da559
Compare
This commit adds the `md-checkbox` component to the material2 repo. It includes unit tests and demos for the component. Note that while there is a demo which exhibits all functionality of the component, the "Nested Checklist" demo is still a WIP. Fixes angular#35
traviskaufman
force-pushed
the
feat-md-checkbox
branch
from
c9da559
to
20876f3
Compare
|
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. |
This commit adds the
md-checkboxcomponent to the material2 repo. Itincludes unit tests and demos for the component. Note that while there
is a demo which exhibits all functionality of the component, the "Nested
Checklist" demo is still a WIP.
Fixes #35