Skip to content
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

Nested <Tearsheet> components not working as intended #2452

Closed
1 of 3 tasks
Pasoon opened this issue Nov 9, 2022 · 21 comments
Closed
1 of 3 tasks

Nested <Tearsheet> components not working as intended #2452

Pasoon opened this issue Nov 9, 2022 · 21 comments
Assignees
Labels
component: Tearsheet Sev 2 Aspects of design is broken and impedes users in a significant way, but has workaround. type: a11y ♿ Issues not following accessibility standards type: bug 🐛 Something isn't working UX: issue Any type of experience feedback that once addressed improves or enhances the experience version: 1 Carbon 10 / v1 version: 2 Carbon 11 / v2

Comments

@Pasoon
Copy link

Pasoon commented Nov 9, 2022

What package(s) are you using?

  • Carbon for IBM Products (or Carbon for IBM Cloud & Cognitive)
  • Legacy/CDAI
  • Legacy/Security

Detailed description

When opening another tearsheet within another tearsheet (nested), you can't interact with any form or input, it does not let you focus or type into the input boxes.

Is this issue related to a specific component?

Tearsheet component

What did you expect to happen? What happened instead? What would you like to
see changed?

I should be able to use nested Tearsheets

What browser are you working in?

Chrome

What version of the @carbon/ibm-products (or @carbon/ibm-cloud-cognitive)
package are you using?

version ^1.32.0

Steps to reproduce the issue

  1. Open tearsheet 1
  2. Open tearsheet 2 that lies within tearsheet 1
  3. Try to type into the input box that is within tearsheet 2

Additional information

Screen.Recording.2022-11-09.at.2.05.07.PM.mov
@Pasoon Pasoon added the type: bug 🐛 Something isn't working label Nov 9, 2022
@elycheea
Copy link
Contributor

@Pasoon Could you provide a reduced case CodeSandbox so we can better investigate this behavior? Our Storybook examples are working so we’d like to better understand how you and your team are using it.

@janhassel
Copy link
Member

@elycheea
Running into the same issue. Here's a quick reproduction: https://codesandbox.io/s/practical-easley-9cd0jm?file=/src/index.js

It happens when you have the second Tearsheet nested inside the first one instead of adjacent to each other.

<Tearsheet title="Tearsheet 1">
  Content for tearsheet 1

  <Tearsheet title="Tearsheet 2">
      Content for tearsheet 2
  </Tearsheet>
</Tearsheet>

@janhassel
Copy link
Member

@elycheea
While restructuring my UI to have the tearsheets adjacent I noticed that the same (or a similar) issue appears when having a Tearsheet adjacent to a CreateTearsheet.

Here is a quick reproduction: https://codesandbox.io/s/upbeat-tess-fi104p?file=/src/index.js

Interestingly enough the focus stealing is fixed whenever you blur the browser and then re-focus.

@elycheea elycheea added UX: issue Any type of experience feedback that once addressed improves or enhances the experience type: a11y ♿ Issues not following accessibility standards Sev 3 Visible or noticeable to users but does not impede functionality. Has a workaround. Sev 2 Aspects of design is broken and impedes users in a significant way, but has workaround. and removed status: open question 💬 Sev 3 Visible or noticeable to users but does not impede functionality. Has a workaround. labels Feb 16, 2023
@Yohanna
Copy link

Yohanna commented Apr 3, 2023

Hi @elycheea, any updates on this issue?

@Yohanna
Copy link

Yohanna commented Apr 11, 2023

Here's a Codesandbox reproducing the nested Tearsheet bug: https://codesandbox.io/s/ibm-products-tearhseet-bug-tc96dw?file=/src/NestedTearsheet.js

Open the second nested Tearsheet, and you'll see that the TextInput field doesn't work.

@elycheea
Copy link
Contributor

Unassigning @matthewgallo for now for. Hoping @davidmenendez or I can take a look at this some more this week.

@Mikadv
Copy link

Mikadv commented Apr 27, 2023

Unassigning @matthewgallo for now for. Hoping @davidmenendez or I can take a look at this some more this week.

Great to hear back from you. Now that is also affecting us and hear from other teams it happens as well. Would be great if you can periodize this further.

@Mikadv
Copy link

Mikadv commented Apr 27, 2023

So we noticed more:

Two things:

  • Tearsheets with no action buttons should not get closed automatically when clicked otuside of it. Carbon modals have a preventCloseOutside prop. This misses for tearsheets.
  • When using stacked tearsheets without action buttons that a click in the stacked tearsheet is regarded as click outside in the first terasheet. Also no option to disabled the click outside behavior...
    • That leads to the bug that any click on the second stacked tearsheet will close the first one

@davidmenendez
Copy link
Contributor

@Pasoon is there a specific reason why the tearsheets have to be nested? we have an example of how to use stacking tearsheets with the code here.

i'll look into this, but just wanted to point this out as an existing implementation.

@davidmenendez
Copy link
Contributor

@Mikadv

  • Tearsheets with no action buttons should not get closed automatically when clicked otuside of it. Carbon modals have a preventCloseOutside prop. This misses for tearsheets.

that's reasonable.

  • When using stacked tearsheets without action buttons that a click in the stacked tearsheet is not regarded as click outside in the first terasheet. Also no option to disabled the click outside behavior...

    • That leads to the bug that any click on the second stacked tearsheet will close the first one

i was not able to replicate this. if i'm understanding this correctly, you're saying that when using stacking tearsheets, if tearsheet 1 has no actions and then you open tearsheet 2, which has actions, that clicking in or out of the tearsheet will close tearsheet 1?

@davidmenendez
Copy link
Contributor

@janhassel i have a feeling this is because create tearsheet has it's own special focus functionality. i'll investigate further.

@Yohanna
Copy link

Yohanna commented May 1, 2023

@Pasoon is there a specific reason why the tearsheets have to be nested? we have an example of how to use stacking tearsheets with the code here.

i'll look into this, but just wanted to point this out as an existing implementation.

Thanks for looking into this, and the code sample, @davidmenendez! In the linked example, the Tearsheets, while visually stacked, are on the same level in the code. Sometimes when we're designing a feature, we might want to separate the Tearsheets into different levels, for better code layout & organization. And to prevent having a single bloated React component containing all the Tearsheets, and the related state...etc.

@Mikadv
Copy link

Mikadv commented May 8, 2023

@davidmenendez

i was not able to replicate this. if i'm understanding this correctly, you're saying that when using stacking tearsheets, if Tearsheet 1 has no actions and then you open Tearsheet 2, which has actions, that clicking in or out of the Tearsheet will close Tearsheet 1?

Yes. If I have stack Tearsheets than clicking outside of Tearsheet 2 closes 2 AND 1. Note that you need to have the Tearsheet nested on INTO another: Say Tearsheet 1 is the parnet component of Tearsheet 2

@elycheea
Copy link
Contributor

@davidmenendez if you’re reaching out to Carbon, please add more detail on this. @matthewgallo shared that there’s a Dialog component now as well ... probably a big change but could be worth considering for the stacking (probably Carbon 11 only).

@elycheea elycheea added version: 2 Carbon 11 / v2 version: 1 Carbon 10 / v1 labels Jun 13, 2023
@davidmenendez
Copy link
Contributor

after the last DSAG meeting it was determined that we need to explore solutions for the nested modal problem. At the request of the other DSAG members i've gone ahead and created a mural for this exploration https://app.mural.co/invitation/mural/watsonassistant2719/1688405516721?sender=dmenend6705&key=ab296ce7-8698-4423-a824-df2c130e2065

@elycheea
Copy link
Contributor

@davidmenendez We’ll put this on hold until we get get some progress through DSAG but should bring this back up on the next call.

@pbthings
Copy link

I just added an example for stacking tearsheets to the Mural, but our developers say that this is not currently possible due to this issue. Is there any update on the fix for this issue? We are urgently trying to deliver this design.

cc: @Ashley-Bock

@laukf
Copy link

laukf commented Oct 11, 2023

Is there any update on this issue esp. progress in allowing focusing on the tearsheet? Besides using the input fields, keyboard events are also not possible which causes further accessibility issues.

@tertau
Copy link

tertau commented Oct 25, 2023

It seems the issue is caused by the focus event bubbling in React tree. If we could stop the event propagation in the nested tearsheet, the focus isn't lost.
As a workaround, we could wrap with div with onFocus to stop propagation. For example, in the case of this comment, it would work by https://codesandbox.io/s/ibm-products-tearhseet-bug-forked-9qmdvt?file=/src/NestedTearsheet.js

<div
  ref={tearsheetWrapperRef}
  onFocus={(e) => {
    if (!tearsheetWrapperRef.current?.contains(document.activeElement)) {
      e.stopPropagation();
    }
  }}
>
  <SecondTearsheet
    isOpen={isSecondTearhsheetOpen}
    onClose={() => setIsSecondTearhsheetOpen(false)}
  />
</div>;

We may not need this workaround when the issue is fixed, but writing here as it might help other people.

@makafsal
Copy link
Member

@elycheea
I think this issue is also resolved when using the useFocus hook #4129 & #4133

https://codesandbox.io/p/sandbox/ibm-products-tearhseet-bug-forked-44r2mx?file=%2Fsrc%2FNestedTearsheet.js

@davidmenendez
Copy link
Contributor

based on the last comment by @makafsal looks like this is now working. closing as complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: Tearsheet Sev 2 Aspects of design is broken and impedes users in a significant way, but has workaround. type: a11y ♿ Issues not following accessibility standards type: bug 🐛 Something isn't working UX: issue Any type of experience feedback that once addressed improves or enhances the experience version: 1 Carbon 10 / v1 version: 2 Carbon 11 / v2
Projects
None yet
Development

No branches or pull requests