-
Notifications
You must be signed in to change notification settings - Fork 298
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[COJ]/account-v2/Likhith/feq-1107/create timeline component #12610
[COJ]/account-v2/Likhith/feq-1107/create timeline component #12610
Conversation
Co-authored-by: “yauheni-kryzhyk-deriv” <“yauheni@deriv.me”> Co-authored-by: Likhith Kolayari <likhith@regentmarkets.com>
❌ Smoke test run (2) failed. See logs for details: Visit Action |
❌ Smoke test run (1) failed. See logs for details: Visit Action |
❌ Smoke test run (2) failed. See logs for details: Visit Action |
❌ Smoke test run (1) failed. See logs for details: Visit Action |
❌ Smoke test run (1) failed. See logs for details: Visit Action |
<div className='flex flex-col items-center'> | ||
{stepCount !== 0 && <StepConnector isActive={isActive} />} | ||
<span className={stepperVariants({ isActive, isFilled: step.isFilled })}> | ||
{step.isFilled ? <StandaloneCheckBoldIcon fill={isActive ? '#fff' : '#000'} /> : null} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will this work ?
{step.isFilled && <StandaloneCheckBoldIcon fill={isActive ? '#fff' : '#000'} />}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it will work (Haven’t tried). I just wanted to be more explicit and handle both flow of if-else
@@ -1,2 +1,4 @@ | |||
dist/*.js | |||
*.config.* | |||
*.classnames.ts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a file called classnames.ts which is a convention used for keeping variant styles. We can sort it but I think sorting for that file is not necessary. But this can be taken for discussion
packages/account-v2/src/components/form-progress/form-progress.classnames.ts
Outdated
Show resolved
Hide resolved
❌ Smoke test run (1) failed. See logs for details: Visit Action |
❌ Smoke test run (2) failed. See logs for details: Visit Action |
❌ Smoke test run (1) failed. See logs for details: Visit Action |
❌ Smoke test run (2) failed. See logs for details: Visit Action |
// [TODO]:Mock - remove once isActive comes from Modal | ||
const updateStep = (index: number) => { | ||
if (steps[index - 1]?.isFilled || index === 0) { | ||
setActiveStep(index + 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we set active step differently in mobile and desktop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The click functions are just mock. To check the behaviour of the component. I have added ToDO before all the lines that needs to be removed or changed
⏳ Generating Lighthouse report... |
Changes:
Displaying the navigation menu on the left side, with clear indications of completed steps, the current step, and the remaining steps, will enhance the customer's ability to visualize and comprehend the flow.