Flow Steps: Vertical variant #254
Conversation
havgry
changed the title
|
I think the vertical variant makes a lot of sense, nice work 👏 I'm not a big fan of having to set height manually. |
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: |
There was a problem hiding this comment.
There needs to be some thoughts put into the spacing and sizing of these bullets.
If these values are "given" by a designer they should be reconsidered.
If possible these should be aligned as well with the grid - if that is not possible (fine) - but we need to address that issue (with element sizing) in the DNA Spec as well.
There are some light improvements to be done Sass wise I'd love to see implemented for clarity's sake
This is added as a way to display all green bullets without breaking the existing behaviour where it's only necessary to apply the "active" class to a single item.
|
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 9e4858e which allows for multiple active elements at the same time (visually, at least) like so:
One could argue that it could be part of a different PR but I'm going to try and sneak it in... |
There was a problem hiding this comment.
I’m okay with introducing this variant, but I would prefer if the distance betweeen the bullets was the same size for UX and design purpose.
Design wise I think it looks better. UX wise I think that some users will think the the size of the step (the distance between the bullets) could mean the time required to fulfill the steps. However, that is only an assumption.
Would flexbox not be a better solution?
| counter-reset: progress-bullet; | ||
| text-align: center; | ||
|
|
||
| #{$variant-vertical-ref} & { |
There was a problem hiding this comment.
I think this is difficult to read.
There was a problem hiding this comment.
It's either that or .ys-flow-steps--vertical & {, I guess. I don't have strong opinions about either one - I just tend to use the former. If you prefer the latter I'm happy to remove the refs.
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. |
havgry
dismissed
dechowdev’s stale review
I'm dimissing this review, as Lucas's comments have either been addressed or fixed and as he's on vacation he's not able to give an approving review.
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:
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):
The reasons for introducing a variant instead of just using media queries are two-fold:
One caveat
If an item spans more than a couple of lines it will break
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:
Thoughts?