feat(step): new variant to support the steps with circular railing #2917

ko2in · 2023-09-28T12:51:36Z

Description

This PR add the new variant railing to the steps which suppose to display the steps with circular railing horizontally or vertically.

I used the name raliing since we already have rail component which can conflict each other. If you have better name for this variant, please suggest.

Testcase

https://jsfiddle.net/ko2in/n9dum7fg/

Screenshot (if possible)

Closes

#2170

ko2in · 2023-09-28T13:16:08Z

I had to fix stylelint errors and rebase to take out this commit, and done forced push. Would it be OK?

lubber-de · 2023-09-28T13:16:58Z

Great! But why dont we just name it circular as we already have for image, icon, label and button?

lubber-de

The inner circle isnt centered, looks misplaced

lubber-de

The inverted variant is not properly supported

Using any other background than white would need to change the non completed icon background as well. I know you set white as background to cover the connection line (i had a similar issue for connected feed)

ko2in · 2023-09-28T13:31:58Z

I didn't implement for inverted yet. I don't know how the inverted should be at the moment.

lubber-de · 2023-09-28T13:34:01Z

The connection line cover background could be avoided by

.ui.railing.vertical.steps .step:not(:last-child) .content::before {
margin-top: 2.2em;
}

it seems....

I wonder why i had so many issues doing that for connected feed... perhaps more difficult markup, Will take a look for that again

ko2in · 2023-09-28T13:37:44Z

Great! But why dont we just name it circular as we already have for image, icon, label and button?

That sound also OK for me. I will update the PR with that name and other fixes.

lubber-de · 2023-09-28T13:41:29Z

I didn't implement for inverted yet. I don't know how the inverted should be at the moment.

Not much to be done i believe. I suggest to:

use dedicated less variables (yes, even if those would have the same value) for the inverted variant
increase specificity, so the normal inverted variant does not override the railing ones (would already fix the inner circle color from black to green)
dont use white as background (perhaps no background at all/transparent in case my margin-top idea works for you would already fix that)
use a light color for the connection lines
use the default inverted green colors

jamessampford · 2023-09-28T13:43:32Z

Nice! Would it be possible to have an option to have a title and/or description on the horizontal variant below the step circles?

lubber-de

The ordered variant is totally messed up unfortunately

lubber-de

The size variants need adjustments at least for vertical variant

tiny steps

check icon too low, but circle icon for uncompleted looks good now :D

massive steps

both icons misplaced
text for content title and description needs some padding (this is also the case for the normal step without your new variant, i just stumbled upon that)

lubber-de

The link step variant isn't supported

lubber-de

The new variant seems to be always unstackable. This could be a design choice and perhaps supported later if a visually pleasing solution can be found

lubber-de

my final comment/suggestion: 😉

disabled step does not have any visual effect (you removed that on purpose...why?)

We should support color names like we have for the connected feed variant.
Currently the green color is hardcoded (@completedColor). The green was supposed to work for the usual step variant where coloring would not make so much sense to me as it would only have effect on the check icon.

But as your new variant concentrates on the icon and connection line there is a possible need for a different color (maybe only for a single completed step) and screams for being colorizable via a color class name.

Support coloring as we have it elsewhere would support

ui purple circular steps
blue completed step
teal step
The connection line should inherit the color as we have it in connected feed

ko2in · 2023-09-28T15:13:08Z

disabled step does not have any visual effect (you removed that on purpose...why?)

I don't see any difference style for disabled step like box item. Maybe, remove the small circle disc?

lubber-de · 2023-09-28T16:19:10Z

disabled step does not have any visual effect (you removed that on purpose...why?)

I don't see any difference style for disabled step like box item. Maybe, remove the small circle disc?

A disabled variant almost always has

a lower opacity value
greyed out (if lower opacity doesn't make it visually clear already (IMHO not necessary when doing the example given above)
pointer-events: none (already implemented)

ko2in · 2023-09-28T16:49:45Z

Disabled state would OK with completed or active. The default state already looks similar to the disabled item as it's color and background is grayed out.

ko2in · 2023-09-28T17:09:46Z

If this looks OK for you, then I'll apply also the opacity for the default state.

lubber-de · 2023-09-28T17:30:51Z

the opacity can be applied to either normal state or completed state. But you should still see the default (non-disabled, non completed) state greyed out (if no color is given as suggested above) and opacity:1 (it gets more clear when text is used in the vertical variant

If you meant the same, then: yes 😄

src/definitions/elements/step.less

ko2in · 2023-10-07T16:56:45Z

@lubber-de Here we go! I've finished the implementation with the other variantions and all your suggestions.

Due to the nature and design approaches, the circular step doesn't have the following variations: stackable, unstackable, attached and evenly divided.

I'm not sure what is going on with jsfiddle.net. It seems to be stopped working for me since yesterday. I cannot browse the website anymore.

So, I've created the examples and demonstrations in CodeSandbox.

jamessampford · 2023-10-07T17:16:15Z

Looking good 😊

The only bit I’m unsure about is the coloured line appearing after the active step, I think it should only go backwards to the previous step (so between completed and active)

Possibly should have an option not to change the header colour as well on the horizontal version (or possibly if by adding .active to the header it inherits the same colour?)

Any possibility of having header supported below the horizontal version?

Not sure if possible at all, but could even be pretty cool to have an optional pulsating active version (of the circle, could possibly have the line between active and last completed pulsating as well separately but certainly the active circle by itself)

lubber-de

👏🏼 This is already very awesome. Your example code is also a good candidate for later docs 😉
Please see my comments again

src/definitions/elements/step.less

src/themes/default/elements/step.variables

src/definitions/elements/step.less

src/themes/default/elements/step.variables

src/definitions/elements/step.less

ko2in · 2023-10-08T08:46:48Z

The only bit I’m unsure about is the coloured line appearing after the active step, I think it should only go backwards to the previous step (so between completed and active)

@jamessampford Yeah! That would be better I guess. What do you think @lubber-de ?

lubber-de · 2023-10-08T10:04:47Z

Yes, agree

ko2in · 2023-10-09T18:39:59Z

Now, the color rule for the rail line is changed as suggested by @jamessampford.

The line only after the completed step is now showing the particular color. The rest of the states would use the default color for the rail line.

Jsfiddle: ~~https://jsfiddle.net/ko2in/86spora2/~~ https://jsfiddle.net/lubber/t1j7yxoz/
CodeSandbox: https://codesandbox.io/s/fui-circular-step-hqc7k7?file=/index.html

jamessampford · 2023-10-09T19:41:37Z

I’ve managed to add optional pulsating to active - https://codesandbox.io/s/fui-circular-step-forked-7tqhs6?file=/index.html - probably needs some tweaking … borrowed from transitions, though needs to target the ::before - I did try ::after as well, but think the former looks better on its own instead of the latter or both

The new variant circular step shows a series of steps with the circular marker and connection line between them. It supports the three layout styles: horizontal, vertical and ordered. It has the same states as box step: active, completed, disabled. And it contains the variations: color, inverted, size. Due to it's design nature, it doesn't have the variations such as: stackble, unstackable, attached and evenly divided.

Only the rail line after the completed step has particular color. The other states use the default color for the rail line.

ko2in · 2023-10-10T09:16:25Z

I’ve managed to add optional pulsating to active - https://codesandbox.io/s/fui-circular-step-forked-7tqhs6?file=/index.html - probably needs some tweaking … borrowed from transitions, though needs to target the ::before - I did try ::after as well, but think the former looks better on its own instead of the latter or both

@jamessampford You can submit your own PR later for any improvements. I would like to keep it as simple for now.

lubber-de

That's very awesome by now!
Just a little comment (see below) and a general question (not a request):

There are now a very lot of :not(.circular) selector enhancements.
Would it be easier (less code?) to instead override the circular setting (which would be needed for non circular only) instead directly. This is especially noticable for all the evenly divided variants (.x.steps)
I understand you did that because circular steps are styled differently, but perhaps it's worth some minutes to think about that again.

So instead of

.ui.steps:not(.circular) {
	display: inline-flex;
}

just override it for the ciscular variant like

.ui.steps {
	display: inline-flex;
}
.ui.circular.steps {
	display: block;
}

src/definitions/elements/step.less

ko2in · 2023-10-12T08:17:35Z

That's very awesome by now! Just a little comment (see below) and a general question (not a request):

There are now a very lot of :not(.circular) selector enhancements. Would it be easier (less code?) to instead override the circular setting (which would be needed for non circular only) instead directly. This is especially noticable for all the evenly divided variants (.x.steps) I understand you did that because circular steps are styled differently, but perhaps it's worth some minutes to think about that again.

So instead of
.ui.steps:not(.circular) {
	display: inline-flex;
}
just override it for the ciscular variant like
.ui.steps {
	display: inline-flex;
}
.ui.circular.steps {
	display: block;
}

I'll try to review again and find out whether it's possible to avoid excluding with :not(.circular) and describe the specific style directly.

lubber-de · 2023-10-12T17:57:57Z

I tried to remove the :not(.circular) for the evenly divided variants (.two.steps, .three.steps etc.) and it seems it works well with circular variant (because it only affects the width), so at least those can be removed i think, but you may check yourself, too

ivangomes · 2024-04-10T20:09:41Z

Guys this project still good?

auto-assign bot requested review from ColinFrick, lubber-de, prudho and y0hami September 28, 2023 12:51

ko2in mentioned this pull request Sep 28, 2023

[steps] circular steps #2170

Open

ko2in force-pushed the feat-railing-step branch from 67a61f1 to 49e8a33 Compare September 28, 2023 13:00

lubber-de reviewed Sep 28, 2023

View reviewed changes

lubber-de added type/feat Any feature requests or improvements lang/css Anything involving CSS labels Sep 28, 2023

lubber-de reviewed Sep 28, 2023

View reviewed changes

lubber-de requested changes Sep 28, 2023

View reviewed changes

lubber-de reviewed Sep 28, 2023

View reviewed changes

lubber-de reviewed Oct 5, 2023

View reviewed changes

src/definitions/elements/step.less Outdated Show resolved Hide resolved

ko2in force-pushed the feat-railing-step branch from 49e8a33 to 77b6906 Compare October 7, 2023 16:46

lubber-de requested changes Oct 7, 2023

View reviewed changes

ko2in added 5 commits October 10, 2023 15:21

Remove unused variables

3ea330e

Fix incorrect variation name

af7e342

feat(step): change the rule about the color of rail line between step

f133607

Only the rail line after the completed step has particular color. The other states use the default color for the rail line.

feat(step): refactor variables and calc() function

89dc439

ko2in force-pushed the feat-railing-step branch from 27877fb to 89dc439 Compare October 10, 2023 08:52

lubber-de reviewed Oct 11, 2023

View reviewed changes

src/definitions/elements/step.less Outdated Show resolved Hide resolved

remove duplicate definitions

a642a81

lubber-de added the state/awaiting-docs Pull requests which need doc changes/additions label Oct 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(step): new variant to support the steps with circular railing #2917

feat(step): new variant to support the steps with circular railing #2917

ko2in commented Sep 28, 2023

ko2in commented Sep 28, 2023 •

edited

lubber-de commented Sep 28, 2023

lubber-de left a comment

lubber-de left a comment •

edited

ko2in commented Sep 28, 2023

lubber-de commented Sep 28, 2023

ko2in commented Sep 28, 2023

lubber-de commented Sep 28, 2023 •

edited

jamessampford commented Sep 28, 2023

lubber-de left a comment

lubber-de left a comment •

edited

lubber-de left a comment

lubber-de left a comment

lubber-de left a comment

ko2in commented Sep 28, 2023

lubber-de commented Sep 28, 2023 •

edited

ko2in commented Sep 28, 2023

ko2in commented Sep 28, 2023 •

edited by lubber-de

lubber-de commented Sep 28, 2023

ko2in commented Oct 7, 2023

jamessampford commented Oct 7, 2023

lubber-de left a comment

ko2in commented Oct 8, 2023

lubber-de commented Oct 8, 2023

ko2in commented Oct 9, 2023 •

edited by lubber-de

jamessampford commented Oct 9, 2023

ko2in commented Oct 10, 2023

lubber-de left a comment

ko2in commented Oct 12, 2023

lubber-de commented Oct 12, 2023

ivangomes commented Apr 10, 2024

feat(step): new variant to support the steps with circular railing #2917

Are you sure you want to change the base?

feat(step): new variant to support the steps with circular railing #2917

Conversation

ko2in commented Sep 28, 2023

Description

Testcase

Screenshot (if possible)

Closes

ko2in commented Sep 28, 2023 • edited

lubber-de commented Sep 28, 2023

lubber-de left a comment

Choose a reason for hiding this comment

lubber-de left a comment • edited

Choose a reason for hiding this comment

ko2in commented Sep 28, 2023

lubber-de commented Sep 28, 2023

ko2in commented Sep 28, 2023

lubber-de commented Sep 28, 2023 • edited

jamessampford commented Sep 28, 2023

lubber-de left a comment

Choose a reason for hiding this comment

lubber-de left a comment • edited

Choose a reason for hiding this comment

lubber-de left a comment

Choose a reason for hiding this comment

lubber-de left a comment

Choose a reason for hiding this comment

lubber-de left a comment

Choose a reason for hiding this comment

ko2in commented Sep 28, 2023

lubber-de commented Sep 28, 2023 • edited

ko2in commented Sep 28, 2023

ko2in commented Sep 28, 2023 • edited by lubber-de

lubber-de commented Sep 28, 2023

ko2in commented Oct 7, 2023

jamessampford commented Oct 7, 2023

lubber-de left a comment

Choose a reason for hiding this comment

ko2in commented Oct 8, 2023

lubber-de commented Oct 8, 2023

ko2in commented Oct 9, 2023 • edited by lubber-de

jamessampford commented Oct 9, 2023

ko2in commented Oct 10, 2023

lubber-de left a comment

Choose a reason for hiding this comment

ko2in commented Oct 12, 2023

lubber-de commented Oct 12, 2023

ivangomes commented Apr 10, 2024

ko2in commented Sep 28, 2023 •

edited

lubber-de left a comment •

edited

lubber-de commented Sep 28, 2023 •

edited

lubber-de left a comment •

edited

lubber-de commented Sep 28, 2023 •

edited

ko2in commented Sep 28, 2023 •

edited by lubber-de

ko2in commented Oct 9, 2023 •

edited by lubber-de