feat: add button-card component #990
Conversation
…into 3pg/feature/button-card-component � Conflicts: � framework/components/AButtonCard/AButtonCard.scss
Codecov Report
@@ Coverage Diff @@
## master #990 +/- ##
==========================================
- Coverage 90.00% 88.90% -1.11%
==========================================
Files 88 89 +1
Lines 2811 2856 +45
Branches 980 991 +11
==========================================
+ Hits 2530 2539 +9
- Misses 240 269 +29
- Partials 41 48 +7
Continue to review full report at Codecov.
|
andreearusu
force-pushed
the
3pg/feature/button-card-component
branch
from
5b6d5e5 to
7ad15fd
Compare
frattaro
changed the title
There was a problem hiding this comment.
Looks good, thanks. In the way of tests, just snapshots should take care of it -- described here: #987 (comment)
Can you add an entry in /framework/changelog.mdx along the lines of:
## 3.2.0 (pending)
- Added `AButtonCard`, `AButtonCardTitle`, `AButtonCardBody` components
| const containerClassName = " a-button-card__title"; | ||
| const className = propsClassName + containerClassName; |
There was a problem hiding this comment.
| const containerClassName = " a-button-card__title"; | |
| const className = propsClassName + containerClassName; | |
| const className = `a-button-card__title ${propsClassName}`.trim(); |
| * String representing class names to be passed to component container | ||
| */ | ||
| className: PropTypes.bool | ||
| }; |
There was a problem hiding this comment.
The proptypes for this component can be removed since it doesn't have any custom props
| const containerClassName = " a-button-card__body"; | ||
| const className = propsClassName + containerClassName; |
There was a problem hiding this comment.
| const containerClassName = " a-button-card__body"; | |
| const className = propsClassName + containerClassName; | |
| const className = ` a-button-card__body ${propsClassName}`.trim(); |
| * String representing class names to be passed to component container | ||
| */ | ||
| className: PropTypes.bool | ||
| }; |
There was a problem hiding this comment.
Same here, this proptypes can be removed
| /** | ||
| * Toggles the `selected` style variant. | ||
| */ | ||
| selected: PropTypes.bool |
There was a problem hiding this comment.
Could add target and value here just because they don't always pass through
| </AButtonCard>`} | ||
| /> | ||
|
|
||
| <Playground |
There was a problem hiding this comment.
This playground needs a heading
| <AButtonCardBody>Description of events goes here</AButtonCardBody> | ||
| </AButtonCard> | ||
| </p> | ||
| </div> |
There was a problem hiding this comment.
| </div> | |
| </div> |
| The `AButtonCard` component inherits passed props. | ||
|
|
||
| <Props of="AButtonCard" /> | ||
|
|
There was a problem hiding this comment.
Need to add props for AButtonCardTitle, AButtonCardBody, even though they're empty, only to explicitly state that they are empty
| $btn-card-min-width: 350px; | ||
| $btn-card-min-height: 150px; | ||
| $btn-card-padding: 20px; | ||
| $btn-card-title-margin-bottom: 10px; | ||
| $btn-card-font-size: $font-size--md; | ||
| $btn-card-bar-height: -5px; |
There was a problem hiding this comment.
Can you please distribute these values and remove the sass variables? They're not set up to be overridden and we don't have guidance on what should be overridable
| $btn-card-body-color: map-get($theme, "btn-card", "body-color"); | ||
| $btn-card-body-color--selected: map-deep-get($theme, "base", "color"); | ||
| $btn-card-border-color--disabled: map-get( | ||
| $theme, | ||
| "btn-card", | ||
| "border-color--disabled" | ||
| ); | ||
| $btn-card-title-color--disabled: map-get( | ||
| $theme, | ||
| "btn-card", | ||
| "title-color--disabled" | ||
| ); | ||
| $btn-card-body-color--disabled: map-get( | ||
| $theme, | ||
| "btn-card", | ||
| "body-color--disabled" | ||
| ); | ||
| $base-color--selected: map-get($theme, "btn-card", "box-shadow--selected"); |
There was a problem hiding this comment.
These also should be distributed
|
I made the following change to the PR description to ensure the issue linking works: -Closes Issue #423
+Closes #423I'm not sure if "Closes Issue" works as an auto-close keyword and I saw GitHub hadn't picked up on the issue link |
Please check if the PR fulfills these requirements
What kind of change does this PR introduce?
Added ButonCard Component
Screen.Recording.2021-10-29.at.16.59.43.mov
Closes #423