feat(shape): Added new Shape APIs #3490
Conversation
|
All 357 screenshot tests passed for commit 6c6ccc1 vs. |
There was a problem hiding this comment.
I don't think there's enough here to justify approving/merging yet. Moreover, I think we should keep this on a feature branch until we have updates for all components, so we avoid pushing a release without all components supported depending on how far we get with this by 0.40.0.
packages/mdc-shape/mdc-shape.scss
Outdated
| .mdc-shape-container__corner--bottom-left { | ||
| transform: rotate(-135deg); | ||
| } | ||
| // Nothing here ¯\_(ツ)_/¯ |
There was a problem hiding this comment.
Is there intended to be anything here eventually, or should we remove this file if it's going to be nothing but mixins?
If the latter, we might also need to make some tweaks to the package check script and the material-components-web package since there will be nothing to include to emit styles for shape.
There was a problem hiding this comment.
Removed this file.
packages/mdc-shape/README.md
Outdated
| @import "@material/button/mixins"; | ||
|
|
||
| .my-custom-button { | ||
| @include mdc-button-shape-radius(pill); |
There was a problem hiding this comment.
This doesn't actually exist in this branch, and I'm unsure OOC whether this is still how pill is expected to work?
There was a problem hiding this comment.
Updated README with relevant usage. PTAL.
packages/mdc-shape/_functions.scss
Outdated
| @function mdc-shape-has-pill-radius($radius) { | ||
| @if type-of($radius) == "list" { | ||
| @each $corner in $radius { | ||
| @if $corner == pill { |
There was a problem hiding this comment.
Does pill make sense at all in list format? Pill applies to both top/bottom corners of a side at once, typically...
There was a problem hiding this comment.
pill just translates to component-height / 2 that is 50%. It is also possible for component to have curve on one side and sharp corners on remaining 3 sides.
packages/mdc-shape/_mixins.scss
Outdated
| $diagonal-length: $size * 1.4142135623730951; | ||
| @mixin mdc-shape-radius($radius, $rtl-reflexive: false) { | ||
| @if mdc-shape-has-pill-radius($radius) { | ||
| @error "Invalid radius: '#{$radius}' is not supported for this component"; |
There was a problem hiding this comment.
How are we supporting pill? Currently it just unconditionally fails here. I'm a little confused since I vaguely recall seeing more to this in previous prototypes.
There was a problem hiding this comment.
This mixin accepts $radius which is absolute value only. Where as resolve-pill mixin accepts radius of absolute and literal.
Simplified this a bit in my following PR.
packages/mdc-shape/_mixins.scss
Outdated
| top: $outline-width; | ||
| border-bottom: $outline-width $outline-style $outline-color; | ||
| @mixin mdc-shape-flip-radius($radius) { | ||
| @if length($radius) == 4 { |
There was a problem hiding this comment.
Shouldn't this also correctly support 2-3?
abhiomkar
force-pushed
the
feat/shape
branch
from
6c6ccc1
to
92fd9a7
Compare
|
Moved this to feature branch. Please see PR #3541 for following changes. |
No description provided.