uiowa / uids

UI Design System
http://uids.brand.uiowa.edu
7 stars 1 forks source link

Refactoring to support downstream work in UIDS Base card updates #847

Closed pyrello closed 1 year ago

pyrello commented 1 year ago

pyrello commented 1 year ago

When testing these updates in SiteNow, we noticed there seems to be an issue with precedence where the media shape classes get precedence over the media size classes and affect whether the sizes actually get rendered correctly.

Behavior can be seen here: http://localhost:6006/?path=/story/components-card--default&args=orientation:left;media_size:small

pyrello commented 1 year ago

We just discussed the issue in https://github.com/uiowa/uids/pull/847#issuecomment-1379131757. It seems that we are running into issues with our media--small, etc. classes having a flex property built into them. After talking it through the media size property doesn't really seem to make a lot of sense. It should really be a property of the component that is implementing a media element. Therefore, we are going to refactor to move this from media into the card. When we eventually want to do the same thing with block quotes, we can abstract it out.

bspeare commented 1 year ago

If we go with the changes in https://github.com/uiowa/uids/pull/847/commits/b5849bec1c69628acee60ca9f88f3bc6802ac377, In SiteNow we would not want to render a media--large class on stacked widescreen cards. If that's not desirable, then we could add a line in for media--large.media--widescreen {width: 100%;} to allow the media--large class to work with stacked cards.

bspeare commented 1 year ago

Since our "small" image size is our default in SiteNow and used in layout columns as lists do we want to set a fixed pixel value for the small size and leave medium and large as percentages? As you can see in these screenshots, small can get very small.

Screen Shot 2023-01-23 at 3 45 37 PM Screen Shot 2023-01-23 at 3 44 43 PM Screen Shot 2023-01-23 at 3 44 30 PM
pyrello commented 1 year ago

Since our "small" image size is our default in SiteNow and used in layout columns as lists do we want to set a fixed pixel value for the small size and leave medium and large as percentages? As you can see in these screenshots, small can get very small.

Maybe we set a min-width for the small size? I'd like to take a look at this using the new grid tool to understand it a bit better. I think we probably need some rules similar to what we developed for inline sizing, but specifically for card images.

bspeare commented 1 year ago

@pyrello that's a good idea!

bspeare commented 1 year ago

Do we want to implement something where we remove the card padding if the background is white against white, gray against gray, etc? Here's an example of where we need it:

https://uiowa.edu/admissions

Screen Shot 2023-01-25 at 12 50 03 PM

https://home.uiowa.ddev.site/admissions

Screen Shot 2023-01-25 at 12 50 18 PM
bspeare commented 1 year ago

Should we add cardbody markup in html tags so that we can target them? In 3.x we had `carddescription` class where we set a custom line-height. Screen Shot 2023-01-25 at 12 52 41 PM

pyrello commented 1 year ago

Should we add cardbody markup in html tags so that we can target them? In 3.x we had `carddescription` class where we set a custom line-height. Screen Shot 2023-01-25 at 12 52 41 PM

Not sure I understand the suggestion here.

bspeare commented 1 year ago

@pyrello marked it as outdated now.

bspeare commented 1 year ago

UIDS 3 Image sizes:

Stacked card

Left/Right aligned:

Current UIDS 4 sizes

Stacked:

Left/Right aligned: Images in small containers are getting squished, need to possibly unset flex property.

V2 paragraphs were using the general sizing mixins (https://github.com/uiowa/uids/blob/3.x/src/components/card/_card-mixins.scss#L135-L154), which is why their sizing looks so different with the UIDS 4 updates.

bspeare commented 1 year ago

It looks like we are adding some properties in https://github.com/uiowa/uids/blob/4.x/src/components/media/media.scss in 4.x that is messing up uids_base media embeds, in particular iframes. Should we scope these to just card media for now?

bspeare commented 1 year ago

Another media.scss issue: https://github.com/uiowa/uids/blob/4.x/src/components/media/media.scss#L5-L8. We don't want to set width: 100%; for our uids_base media images. Like my previous comment, should we just scope these media rules to the card?