fix(tile): compact link icon layout #2082
Conversation
🦋 Changeset detectedLatest commit: 56d8433 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: -149 B (-0.07%) Total Size: 207 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 |
There was a problem hiding this comment.
Placement of the icon in compact tiles looks good!
-
Is the icon in this tile (seen in color context demo) showing how to add an icon inline with the body text?
-
For the regular tiles with an icon, I'm seeing a 55px height instead of the expected 48px (based on the icon size). It's making the space between the icon and title look too big. Would a
max-heightfor the icon slot help here?
I'm not sure what you're referring to, there are 27(!) example tiles in that file. Can you be more specific, please?
Is that an issue of demo content? after merging in main I believe this issue is resolved: https://deploy-preview-2082--red-hat-design-system.netlify.app/elements/tile/demo/desaturated-heading/ |
Argh, I think I meant to add a screenshot. It's this one: |
It seems like the height is different depending on whether you're looking at the icon slot under shadow DOM light DOM |
seems likely to me this was an oversight - i added the |
See above, the icons and title line up on screen. I believe the discrepancy you're noticing is between the slotted svg element and the slot element it projects into, which have different styles. |
@marionnegp This still needs design clarification: is the icon always supposed to have see #2082 (comment) |
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