carbon-design-system / ibm-products

A Carbon-powered React component library for IBM Products
https://ibm-products.carbondesignsystem.com
Apache License 2.0
94 stars 137 forks source link

[CreateTearsheet]: including steps dynamically with includeStep not working after v2.36 #5813

Closed Tomahaawk closed 1 month ago

Tomahaawk commented 1 month ago

Package

Carbon for IBM Products

Description

We are using CreateTearsheet component with a couple of steps being included/excluded based on an option the user selects on a previous step.

It was working fine until v2.35, and then it broke after we upgraded to v2.45. After going through the changelogs, I noticed a change on v2.36 that includes a state change inside an useEffect that resets the tearsheet step to the first step, which I think is causing the problem.

I added a stackblitz reproducing the issue. If you downgrade ibm-products to v2.35 you can see the form works as expected.

Component(s) impacted

CreateTearsheet

Browser

Chrome, Firefox

@carbon/ibm-products (previously @carbon/ibm-cloud-cognitive) version

v2.45

Suggested Severity

Severity 2 = Aspects of design is broken, and impedes users in a significant way, but there is a way to complete their tasks. Affects major functionality, has a workaround.

Product/offering

IBM RPA

CodeSandbox or Stackblitz example

https://stackblitz.com/edit/github-kndrwa?file=src%2FApp.jsx

Steps to reproduce the issue (if applicable)

1 - Create a form using CreateTearsheet and set a couple of steps to be included dynamically 2 - Include an intro step and a step with an option selection that is saved in a state 3 - The following steps should be included based on the option selected (using includeStep prop) 4 - Open the tearsheet and try to select the option

Expected result: user should be able to go to the next step Actual result: tearsheet goes back to the first step

Release date (if applicable)

No response

Code of Conduct

davidmenendez commented 1 month ago

@Tomahaawk i think the problem here is that the flow is expected to be the introStep which is where typically you would establish any of the dynamic steps. i believe this is the intended behavior, but i don't see anything that explicitly forbids this in the create flow documentation. i will get design input on this and confirm whether or not we want to specifically allow steps to be dynamically set outside of the designated introStep

i did test it locally and confirmed that adding introStep to the second step does alleviate this problem. if you need a temporary solution i believe that will work.

Tomahaawk commented 1 month ago

i think the problem here is that the flow is expected to be the introStep which is where typically you would establish any of the dynamic steps.

Just to give more context, in our use case we actually have the intro step where the user selects a flow, then in the middle of the flow there is another step where the user selects an option to decide what the following step will be. We have 2 steps that can change the flow of the form.

1pharaxh commented 1 month ago

@davidmenendez We are using the CreateTearsheet component in IBM Guardium Insights to dynamically add steps based on options a user selects. The issue is due to the addition of the else statement in v2.36 the CreateTearsheet component resets after a step is added through the includeStep prop and the tearsheet goes to the first step. Here is a StackBlitz for the same the intended behaviour is to proceed to the next step after adding steps through the checkboxes but instead we get redirected to the first step. This is blocking our efforts of updating Guardium Insights to the latest Carbon Version.

davidmenendez commented 1 month ago

@1pharaxh @Tomahaawk i have a fix for this and will submit shortly.

1pharaxh commented 1 month ago

Thanks for the fix @davidmenendez is this going to be in the next release ?

davidmenendez commented 1 month ago

@1pharaxh i'm hoping it will. still waiting for it to be reviewed and merged. will you you posted.

davidmenendez commented 1 month ago

@1pharaxh @Tomahaawk the fix was just committed and should appear in the next release. if you have any other questions please feel free to follow up. thanks again for opening an issue for this defect 👍