feat(shape): Added new Shape APIs #3541
Conversation
williamernest
force-pushed
the
feat/shape_api
branch
from
8094aea
to
d68700d
Compare
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
|
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) |
|
CLAs look good, thanks! |
packages/mdc-shape/_functions.scss
Outdated
| @function mdc-shape-prop-value($radius) { | ||
| @if type-of($radius) == "list" { | ||
| @if length($radius) > 4 { | ||
| @error "Invalid radius: '#{$radius}' radius is not supported"; |
There was a problem hiding this comment.
This error message could be more specific for what it's being raised for
Codecov Report
@@ Coverage Diff @@
## feat/shape #3541 +/- ##
=============================================
Coverage ? 98.41%
=============================================
Files ? 120
Lines ? 5036
Branches ? 620
=============================================
Hits ? 4956
Misses ? 80
Partials ? 0Continue to review full report at Codecov.
|
|
Updated this branch to support percentage value for |
packages/mdc-shape/README.md
Outdated
| @@ -36,74 +35,50 @@ applying angled corners to unelevated surfaces. | |||
| npm install @material/shape | |||
| ``` | |||
|
|
|||
| ## Basic Usage | |||
| ## Usage | |||
There was a problem hiding this comment.
Discussed offline: let's bring this back in line with the readme standards/template.
|
All 389 screenshot tests passed for commit 3a3afc6 vs. |
packages/mdc-shape/_functions.scss
Outdated
|
|
||
| @function mdc-shape-resolve-percentage-for-corner_($component-height, $radius) { | ||
| @if type-of($radius) == "number" and unit($radius) == "%" { | ||
| // Converts the percentage to number without unit. Example: 50% => 50. |
There was a problem hiding this comment.
My request for a comment here was to explain why this works, not what it's doing. Add another line here:
// This is accomplished by dividing the value by itself to cancel out units, while resulting in a denominator of 1.
There was a problem hiding this comment.
I feel documenting what this accomplishes is important than explaining implementation details. Developer would figure out how this works once he know what it does.
There was a problem hiding this comment.
In general that's probably true, but this is basically taking advantage of specific Sass behavior to workaround what basically amounts to a missing API. I was able to surmise what it accomplished almost immediately, but it looked mysterious and took me a minute to rationalized why it worked. My request was to save others that minute.
There was a problem hiding this comment.
Moved this to a private function so it won't clobber the code.
|
All 389 screenshot tests passed for commit b3067cd vs. |
|
All 389 screenshot tests passed for commit b5dc384 vs. |
|
All 389 screenshot tests passed for commit 6241ada vs. |
|
All 389 screenshot tests passed for commit b55ee00 vs. |
Previously on PR #3490
Fixes #3175
BREAKING CHANGE: The previous contents of the mdc-shape package have been removed and replaced with mixins implementing the Shape system. This system implements only rounded corners to provide a straightforward CSS-only solution.