search

youseedk / dna

CSS framework for yousee.dk
ISC License
6 stars 6 forks source link

Flow Steps: Vertical variant #254

Closed havgry closed 4 years ago

havgry commented 4 years ago

This has been bugging me for quite some time - the labels in the flow steps component are hidden below a certain viewport width like so:

Screenshot 2020-07-07 at 21 56 38

This renders the component pretty useless for seeing people not relying on the aria attributes. I propose we introduce a vertical variant (I even believe Bo designed something similar):

Screenshot 2020-07-07 at 21 57 08

The reasons for introducing a variant instead of just using media queries are two-fold:

  1. Avoiding introducinging breaking changes in the existing component
  2. Leaving it up to the designer and developer when to use what version

One caveat

If an item spans more than a couple of lines it will break

Screenshot 2020-07-07 at 21 58 54

This is because the connectors are built on the assumption that the distance between each bullet always is the same. Two possible solutions, as I see it: Always set a height on the container and let flexbox distribute the items evenly (easy), or change the connectors so they allow for various heights/widths of each item (a bit more work). That way we would also be able to do things like:

Screenshot 2020-07-07 at 22 51 25

Thoughts?

Nickhoyer commented 4 years ago

I think the vertical variant makes a lot of sense, nice work 👏 I also think long text won't look good on these, even if the connectors allowed it. It should lie on the content editors to make short labels for them.

I'm not a big fan of having to set height manually.

havgry commented 4 years ago

I also think long text won't look good on these, even if the connectors allowed it.

I agree, but I don't think the component should break when easily avoidable. If we could agree to always use the same connector background color we could prevent it from breaking and simplify the code at the same time. Right now there's a tiny difference in color, I only noticed when I started working on the component:

Screenshot 2020-07-08 at 14 22 46

It would still be possible with the color difference, but would require a bit more CSS.

havgry commented 4 years ago

Design: I've talked it over with Bo and he is ok with the change. We also talked about font weights which we will leave as is for now.

I've adressed all of @Dechowmedia's points (thanks for the review btw). I'm still looking for specific feedback regarding the caveat mentioned about multiple lines of text.

Also, I've introcued a small change in https://github.com/youseedk/dna/pull/254/commits/9e4858ece5d4e04c5f2057395ea435cb8a7d6427 which allows for multiple active elements at the same time (visually, at least) like so:

BB_mobil_delivery_collapsed@2x

One could argue that it could be part of a different PR but I'm going to try and sneak it in...

havgry commented 4 years ago

Would flexbox not be a better solution?

It doesn't make much difference. In my proposal there's a minimum height for each item. If you wanted to automatically distribute the items with flexbox, you'd still need an explicit or implied height on the container - just like you have with the width on the horizontal variant. So basically we would have to require a height on the container. In that case I would rather set a specific height for each item allowing for at least two lines of text.