search

uiowa / uids

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

Finish card component #785

Closed joewhitsitt closed 1 year ago

joewhitsitt commented 1 year ago

Resolves https://github.com/uiowa/uids/issues/781

To test

Remaining tasks

Potential additional tasks

Notes for SiteNow

bspeare commented 1 year ago

I think we still need card__meta: https://github.com/uiowa/uids/pull/785/files#diff-8e038ebe0a97c988224576ad941b3eac4a9fa27cf477e105f6d4d7ca81ba98c8L129-L138

bspeare commented 1 year ago

I'm looking to add the container queries polyfill:

<script type="module">
  if (!("container" in document.documentElement.style)) {
    import("https://unpkg.com/container-query-polyfill@^0.2.0");
  }
</script>

https://storybook.js.org/docs/react/addons/writing-presets#previewmanager-templates https://storybook.js.org/docs/react/configure/story-rendering#adding-to-head

GaryRidgway commented 1 year ago

Checking this, I am seeing that Herky is being squashed on person profile cards. I talked to @bspeare and it seems like we will normally have a 1x1 aspect ratio for these images, but is it desirable for us to do some sort of object fit so that people who are not using images styles à la Drupal can still just slap in whatever photo they want?

Example here Screen Shot 2022-10-21 at 11 17 14 AM
GaryRidgway commented 1 year ago

Seeing how we are re-organizing things in this issue, I would love to have a pass that we go through to re-title prop and slot labels and organize them in to more reasonable groupings for controls. Just a few UX/consistency updates can go a long way

joewhitsitt commented 1 year ago

is it desirable for us to do some sort of object fit

I think if this is to be used by folks outside of Drupal, it would be good to have. Not sure how that conflicts with https://github.com/uiowa/uids/issues/804 though.

makurj commented 1 year ago

Could we switch the "Display Options > No border" to "Display Options > Border > True/False"? The double negative could cause confusion.

makurj commented 1 year ago

Circle image doesn't seem to work with horizontal orientation (left or right), unless you set the circle image first, but then orientation doesn't work.

image

Stacked, circle, padded or not: image size doesn't change between medium and large.

image image

makurj commented 1 year ago

No corresponding "collection" items found.

e-marie-w commented 1 year ago

If nobody minds another set of eyes I have some free time and can also look at this.

makurj commented 1 year ago

Tried circle image with left and right orientation. Didn't seem to work:

image

image

Noticed similar behavior with other images crop with left/right orientation. It didn't seem to switch.

bspeare commented 1 year ago

Stacked, circle, padded or not: image size doesn't change between medium and large.

When circle image is set within "stacked", the media—padded class needs to be applied. I assume we will be adding the media--padded class programmatically in SiteNow when that selection is made?

bspeare commented 1 year ago

Tried circle image with left and right orientation. Didn't seem to work:

@makurj Try changing the viewport to "Tablet", all mobile styles are forced into the stacked format currently.

Screen Shot 2022-10-26 at 9 15 11 AM
makurj commented 1 year ago

That worked! Thanks, Ben.

makurj commented 1 year ago

"Image Using Font Awesome Icon": The icon disappears on "large mobile" with any shape other than "widescreen", no matter the orientation. It appears fine for tablet viewport.

bspeare commented 1 year ago

"Image Using Font Awesome Icon": The icon disappears on "large mobile" with any shape other than "widescreen", no matter the orientation. It appears fine for tablet viewport.

I'd be in favor of hiding this story for now. I don't think this story is finished and it's not something we are using in SiteNow currently.

e-marie-w commented 1 year ago

Could we switch the "Display Options > No border" to "Display Options > Border > True/False"? The double negative could cause confusion.

So this did confuse me! Especially because a different section has Media > Border > True/False. I'm thinking we set up our default card so that all extra options are False. And since we wanted a card border but no media border for default, the card border option ended up being No border set as False and media border just Border set as False. So, it makes sense why we did it this way, but I still think it's confusing to users (both the double negative and the options not matching) and worth discussing changing.

e-marie-w commented 1 year ago

Not sure this is in scope, as it wasn't a combination option on the old UIDS site, but I wanted to document it for the future (assuming it isn't expected behavior).

When I turn on Align button to bottom and Centered when in tablet mode or have reset the viewport, the centering no longer takes effect. It varies between the two - reset viewport (computer view I assume?) aligns everything to the left. In tablet view, the content remains centered, but the title and link are left-aligned. This happens in all orientations - left, right, and stacked.

Also happening with the above combinations is that the subtitle and meta text behave strangely when both added. Subtitle by itself is left-aligned like the title and link. But when I start typing meta text the subtitle starts moving to the right. Meta text is left-aligned with and without the subtitle.

Edit: I figured out screenrecording! So you can see it instead of having to recreate it yourself.

https://user-images.githubusercontent.com/68179015/198085569-b91f4c87-1627-4cda-acd5-95504158248a.mov

e-marie-w commented 1 year ago

Not in scope and not an error or loss of functionality but a change that may cause confusion: if the card has link text the Props category appears before Display Options. When you remove link text it appears after Display Options. aka I thought I broke something until I scrolled down.

joewhitsitt commented 1 year ago

pulling down

joewhitsitt commented 1 year ago

@media (min-width: 768px) .card--button-align-bottom .card__body seems to override text-align: center; with align-items: flex-start;

should this be a :not somehow?

bspeare commented 1 year ago

@joewhitsitt I think we need to set a default margin at https://github.com/uiowa/uids/blob/f4fadd1bdedcdf2c0fd28713ec52301fef631574/src/components/card/card.scss#L89 on the bttn to something like:

footer {
    .bttn {
      margin-top: 2rem;
    }
  }

Then this would work. I liked targeting the <footer> for the margin on this but I think we need a default height set on the bttn.

joewhitsitt commented 1 year ago

@bspeare i made that margin change, but there is still this alignment difference between 3.x and 4.x:

Screen Shot 2022-10-26 at 4 08 14 PM

https://uids.brand.uiowa.edu/components/detail/card--centered-enclosed.html

bspeare commented 1 year ago

@joewhitsitt can you share a link that contains the args in that screenshot?

bspeare commented 1 year ago

I think I found it: http://localhost:6006/?path=/story/components-card--person-profile&args=button_align_bottom:true;borderless:false;centered:true;orientation:

bspeare commented 1 year ago

@joewhitsitt I think https://github.com/uiowa/uids/pull/785#issuecomment-1292655784 is fixed by moving the <footer> outside of card--body.

joewhitsitt commented 1 year ago

This might have introduced a new oddity:

http://localhost:6006/?path=/story/components-card--default&args=button_align_bottom:true;orientation:left;media_border:true;media_shape:circle;media_padded:true

Screen Shot 2022-10-26 at 4 08 14 PM
joewhitsitt commented 1 year ago

As a team we reviewed the feedback that was part of this PR and have fixed the items in scope.