feat(tile, tile-group): add components #12091
Conversation
andy-blum
requested review from
m4olivei,
bruno-amorim,
marcelojcs and
Valentin-Sorin-Nicolae
and removed request for
a team
✅ Deploy Preview for ibm-dotcom-web-components ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for ibm-dotcom-web-components-react-wrap ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
andy-blum
added
the
Needs design approval
andy-blum
requested review from
areagan1030
and removed request for
bruno-amorim,
marcelojcs and
Valentin-Sorin-Nicolae
andy-blum
added
Needs design approval
There was a problem hiding this comment.
Noticed a couple of things from a static review perspective. Also, review CI jobs, looks like at least stylelint is angry.
Also comparing the componets on CAEM vs here:
The image ratios are different. Not sure which is correct:
The Tile Group Default and With Image stories lost their knobs configuration.
| * LICENSE file in the root directory of this source tree. | ||
| */ | ||
|
|
||
| import { html } from 'lit-element'; |
There was a problem hiding this comment.
We should take the opportunity here to update lit imports to the recommended form. See the v2 upgrade guide. I had a PR open to do this on CAEM, never got around to tidying it up, but it stands as a good guide to the changes we need. Doing this change will align well with existing components in the repo.
| focusable="false" | ||
| preserveAspectRatio="xMidYMid meet" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| data-autoid="dds--card__pictogram" |
There was a problem hiding this comment.
dds prefix here. We can probably just drop this data attribute I'd think? Was probably copy 🍝 from somewhere long ago.
| // @use '@carbon/styles/scss/type' as *; | ||
| // @use '@carbon/styles/scss/theme' as *; | ||
| // @use '@carbon/styles/scss/utilities'; | ||
| // @use '@carbon/styles/scss/spacing' as *; | ||
| // @use '@carbon/styles/scss/reset' as *; | ||
| // @use '@carbon/grid' as *; | ||
| // @use '@carbon/ibmdotcom-styles/scss/globals/utils/flex-grid' as *; |
There was a problem hiding this comment.
Remove commented out code.
| content: ''; | ||
| display: block; | ||
| /* stylelint-disable-next-line property-no-unknown */ | ||
| aspect-ratio: var(--caem-tile-aspect-ratio, 16 / 9); |
There was a problem hiding this comment.
Any reason to keep the caem prefix here, or can we update this to a c4d prefix?
| */ | ||
|
|
||
| import { LitElement, html, property, state, query } from 'lit-element'; | ||
| import { classMap } from 'lit-html/directives/class-map.js'; |
There was a problem hiding this comment.
Any directive imports also need updating to use the lit package instead.
Related Ticket(s)
https://jsw.ibm.com/browse/ADCMS-6691
Description
Migrates the
tileandtile-groupcomponents fromcarbon-for-aemintocarbon-for-ibmdotcomChangelog
New
c4d-tilec4d-tile-group