fix(tile): compact link icon layout #2082
Conversation
🦋 Changeset detectedLatest commit: 23d2be7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for red-hat-design-system ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Size Change: -251 B (-0.12%) Total Size: 208 kB
ℹ️ View Unchanged
|
|
@bennypowers I think we need to update many of the demos. There doesn't seem to be an example of both |
There was a problem hiding this comment.
- Just for the prettiness of the demo, can you fix the broken icon in the color context and compact demos?
-
Probably nitpicky, but can the blue icons in the icon demo and the link types demo use
--rh-color-icon-primary-on-lightinstead? -
For the default size tile, the space between an icon and a title should be
--rh-space-2xl. Currently looks like there's no space in the icon demo
-
In selectable tile, the required slot is the header, so in the form control demo I'd just move the placeholder text that's in the body.
|
for the icon and link-types demo, where the icon attribute is used, the color for the icon is defined as
|
|
Regarding the form control demo, I believe the guidelines docs are out of date. They state: Writing contentThe four content slots within a tile are title, heading, body, and footer.
however in the element definition, we have /**
* @slot image - optional image on top of tile
* @slot icon - optional icon
* @slot title - optional title
* @slot headline - optional headline / link title
* @slot - optional body content
* @slot footer - optional footer
*/which accurately lists the available slots, but does not document their use and purpose as well as the guidelines pages. So there's a mismatch here that needs to be addressed. I attempted to resolve this by copying the guidelines text into the jsdoc, slightly modified (since compact tiles also don't have title slots). I also changed the guidelines page to refer to headline instead of heading I think this points to a more pervasive issue, being the disconnect between developer-focused "code" api docs and designer-focused guideline docs. |
|
Regarding selectable tile content in the demos: i wrapped the content in Before <rh-tile-group radio>
<rh-tile name="radio" value="1"><span slot="headline">Radio 1</span></rh-tile>
<rh-tile name="radio" value="2"><span slot="headline">Radio 2</span></rh-tile>
</rh-tile-group>
<rh-tile-group checkable>
<rh-tile name="check" value="1"><span slot="headline">Check 1</span></rh-tile>
<rh-tile name="check" value="2"><span slot="headline">Check 2</span></rh-tile>
</rh-tile-group>After: <rh-tile-group radio>
<rh-tile name="radio" value="1" headline="Radio 1"></rh-tile>
<rh-tile name="radio" value="2" headline="Radio 2"></rh-tile>
</rh-tile-group>
<rh-tile-group checkable>
<rh-tile name="check" value="1" headline="Check 1"></rh-tile>
<rh-tile name="check" value="2" headline="Check 2"></rh-tile>
</rh-tile-group> |
|
Layouts here appear to be broken, will have to fix them before merging |
What I did
<rh-tile compact>when there is a link and an iconTesting Instructions
compact-link-with-icondemo (and othersj., ensure it conforms to figma specsNotes to Reviewers
We might want to implement other patterns from the figma here, like compact with image and icon, different sized icons, etc. See figma
Thanks @tejas161 for reporting this issue