react-modal icon indicating copy to clipboard operation
react-modal copied to clipboard

Tabbing does not work inside modal

Open mitchdowney opened this issue 6 years ago • 27 comments

Summary:

Not sure if this is a bug or if I messed something up. The problem is that I can't reproduce the bug on localhost, and can only reproduce it in prod :/

Tabbing does not work inside the modal. The modal never gains focus, and after I click inside inputs, pressing tab has no effect.

I'm using OSX Catalina, and the same issue happens in Chrome 77 and Firefox 67.

Steps to reproduce:

(Sorry I don't have time for a proper demo right now. I can possibly revisit this again to create a demo someday.)

  1. Go to podverse.fm
  2. Click the dropdown icon in upper-right corner of the navigation
  3. Click Login to trigger the modal
  4. The modal does not take on focus, and pressing tab has no effect in the modal.

The code for this modal can be found here.

Expected behavior:

Expected the modal to take focus once it is shown, and for tab to be able to navigate between the focusable elements. Also expect this behavior to be consistent on both localhost and prod.

mitchdowney avatar Oct 28 '19 03:10 mitchdowney

Figured out my issue. I had the modal inside a wrapper, and I gave that wrapper position: absolute; z-index: 101; because I wanted the modal and the wrapper/backdrop to be positioned over other elements on the page. That resulted in the wrapper having 0 for height and width. I saw in recent commit #774 that scrollHeight is somehow needed by tabbable, so a wrapper with 0 height/width seemed like a problem, so I tried giving the wrapper the following rules:

    position: absolute;
    z-index: 101;
    height: 100vh;
    width: 100vw;

And the modal began accepting tab focus after that. I'll go ahead and close this issue.

mitchdowney avatar Oct 28 '19 03:10 mitchdowney

Actually I'll reopen in case this is a new behavior you want to track.

mitchdowney avatar Oct 28 '19 04:10 mitchdowney

Thanks for writing this issue.

I just had the same problem because we are using a predefined Portal container for all our Modals, which has always been a div without any height or width and tabbing stopped working on the latest version, and now I understand why 😄

jonnybel avatar Nov 26 '19 16:11 jonnybel

Can confirm this was introduced in v3.11.1 Rolling back to v3.10.1 resolves the issue

If you happened to have some portal wrapper this can cause a pretty serious accessibility defect.

Indimeco avatar Nov 28 '19 07:11 Indimeco

Hi guys, just wondering if there's any plan to look into this as I'd say it's pretty commonplace to wrap react-modal to have a reusable Dialog component? And adding height to the wrapper isn't really a solid solution.

chris-pearce avatar Jan 17 '20 02:01 chris-pearce

same here: i had position: fixed; z-index: 1100; and tab button didnt work. Rolling back to v3.10.1 resolves the issue

taipignas avatar Feb 12 '20 11:02 taipignas

I encountered this issue in v3.11 as well. Not sure what the best solution is but tabbing is crucial for me since I have a lot of modals with forms.

willsmanley avatar May 25 '20 07:05 willsmanley

Possibly related - sometimes I use 2 modals that layer on top of each other. Even when I roll back to v3.10.1, the tabbing doesn't work in the 2nd modal UNLESS there is an input field somewhere in the 1st modal. Something like this produces the issue in v3.10.1:

<Modal isOpen/>
    <input/>
    
    <Modal isOpen/>
        // tabbing works here
        <input/>
        <input/>
    </Modal>

</Modal>
<Modal isOpen/>    

    <Modal isOpen/>
        // tabbing does NOT work here
        <input/>
        <input/>
    </Modal>

</Modal>
<Modal isOpen/>    
    <input style={{display: 'none'}}/>

    <Modal isOpen/>
        // tried this as a hacky workaround but even this does not work
        <input/>
        <input/>
    </Modal>

</Modal>
<Modal isOpen/>    
    <input style={{visibility: 'hidden', position: 'absolute'}}/>

    <Modal isOpen/>
        // finally this workaround does work without affecting styles
        <input/>
        <input/>
    </Modal>

</Modal>

willsmanley avatar May 25 '20 07:05 willsmanley

@willsmanley can you create a repo with this examples, please? It will make it easy to debug...

diasbruno avatar May 25 '20 13:05 diasbruno

I can confirm that reverting to 3.10.1 fixes the tabbing issue. The issue persists in 3.11.2.

c10b10 avatar Oct 16 '20 15:10 c10b10

@diasbruno here's a reproduction of this issue:

https://github.com/podverse/sandbox/tree/react-modal/issue-782/demo

You'll need to be on the react-modal/issue-782 branch for the demo.

mitchdowney avatar Oct 21 '20 00:10 mitchdowney

I had the same issue and was not able to get other solutions working quickly, so I came up with a brute force approach. Make a ref to the container element that holds the focusable elements that you wish to make tabbable.

import React, { useRef} from 'react';
import ReactModalTabbing from '../../yourpath';

const YourComponent = (props) =>{
const formRef = useRef();
return (
<ReactModalTabbing containerRef={formRef}>
        <form ref={formRef} onSubmit={handleSubmit}  >
            <input type="text" />
            <input type="text" />
            <input type="text" />
            <input type="text" />
        </form>
  </ReactModalTabbing>
);
}

And this is the component

import React, { useState, useEffect } from 'react';

const ReactModalTabbing = ({ containerRef, children }) => {
    const [configuredTabIndexes, setConfiguredTabIndexes] = useState(false);

    const focusableElements = () => {
        // found this function body here https://zellwk.com/blog/keyboard-focusable-elements/
        return [...containerRef?.current?.querySelectorAll(
            'a, button, input, textarea, select, details, [tabindex]:not([tabindex="-1"]):not([type="hidden"])'
        )].filter(el => !el.hasAttribute('disabled'));
    }

    const isTabbable = (element) =>{
        if(element.getAttribute('tabindex')){
            return true;
        }
        return false;
    }

    const findElementByTabIndex = (tabIndex) => {
        return containerRef?.current?.querySelector(`[tabindex="${tabIndex}"]`);
    }
    
    const moveFocusToTabIndex = (tabIndex) => {
        findElementByTabIndex(tabIndex)?.focus();
    }

    const handleKeyDownEvent = (event) => {
        if(!isTabbable(event.target)){
            return;
        }

        const tabIndex = parseInt(event.target.getAttribute('tabindex'));

        if(event.shiftKey && event.key === 'Tab'){
            moveFocusToTabIndex(tabIndex - 1);
        }else if(event.key === 'Tab'){ //should probably make sure there is no other modifier key pressed.
            moveFocusToTabIndex(tabIndex + 1);
        }
    }

    useEffect(() => {
        if(!configuredTabIndexes && containerRef.current){
            setConfiguredTabIndexes(true);
            focusableElements().forEach((el, index) => el.setAttribute('tabindex', index + 1));
            containerRef?.current?.addEventListener('keydown', handleKeyDownEvent);
        }
    });

    return children;
}

export default ReactModalTabbing;

paulewetzel avatar Feb 19 '21 06:02 paulewetzel

Caused by zero-sized div.ReactModalPortal as it's child has position: fixed. The culprit is tabbable helper that thinks that all inputs inside the modal are invisible and disables tab key behavior.

To fix, set .ReactModalPortal style to:

position: absolute;
height: 1px;
width: 1px;

tpacent avatar Mar 11 '21 16:03 tpacent

In my case, adding event.stopPropagation() above scopeTab here resolves the issue: https://github.com/reactjs/react-modal/blob/68af7ecdd79993dfcc1508587f1e145465f28f85/src/components/ModalPortal.js#L278

I noticed that it is calling the tabbable helper twice to get all tabbable components, the first call is retrieving all tabbable elements properly, and the second one is returning a completely empty list. Stopping the propagation for the event allows for me to tab properly in the modal.

stshort avatar Feb 24 '22 00:02 stshort

@stshort That's interesting. stopPropagation() is hard to use in this case because we need to be careful to not stop the browser's behavior.

Can you give more context about this double call, please?

diasbruno avatar Feb 24 '22 17:02 diasbruno

@diasbruno I actually was digging at this a bit more, and found some of the old code in a ModalFactory component I wrote a couple years back as I was generalizing Modal calls was inadvertently instantiating nested 2 ModalPortal instances in this one specific modal component.

This makes a lot of sense now, I was quite confused as to why there were two instances being called, but yeah this is definitely the issue.

stshort avatar Feb 24 '22 23:02 stshort

That's great, @stshort. But, maybe there is something related to something like this...

The inner modal receives the click, but since the outer modal is also open, the handler is getting the click (?)

<modal>
   <modal />
</modal>

diasbruno avatar Feb 24 '22 23:02 diasbruno

@diasbruno Yeah I was doing something like this:

      const ModalComponent = ModalUtil.getModal(modalType);
      modalDisplay = (
        <Modal isOpen={showModal} onRequestClose={this._onCloseModal} style={modalStyles}>
          <ModalComponent {...modalProps} />
        </Modal>
      );

Where <ModalComponent> was ultimately calling:

    ModalUtil.ModalFactory(
      modalProps.onCloseModal,
      <React Component for Modal />,
      MODAL_TYPE_IDENTIFIER_CONSTANT
    );

And the ModalFactory method was:

  static ModalFactory = (onRequestClose, modalComponent, modalType) => (
    <Modal isOpen={true} onRequestClose={onRequestClose} style={ModalUtil.getModalStyle(modalType)}>
      {modalComponent}
    </Modal>
  );

So, yeah, as you could see, I was inadvertently calling <Modal> twice in this one case, which ended up with a DOM tree similar-ish to what you were mentioning. It's odd because everything else regarding the Modal's functionality worked completely fine, except in this one case since it was propagating upwards, the logic in the tabbable helper for the parent component detected no tabbable elements. All of the input fields and everything I had on the inner modal worked fine otherwise, it just seems that the onKeyDown event propagated upwards to the outer modal.

stshort avatar Feb 24 '22 23:02 stshort

it just seems that the onKeyDown event propagated upwards to the outer modal.

This is what is difficult to handle in this case. It shouldn't have propagated (maybe the synthetic event since they are in the same path).

diasbruno avatar Feb 24 '22 23:02 diasbruno

Yeah, to be honest, I'm not sure why it was propagated upwards. I briefly looked through the Modal.js code and ModalPortal.js code and in my codebase I can confirm that we are using React 16+ (16.13.1 specifically), so we are definitely using the native React.createPortal for instantiating the portal(s). I'm not familiar enough personally with creating React portals to know what the correct behavior would be when having nested portals, but guess that they will bubble upwards within the React tree in this case, at least this article is also stating this to be the case:

https://jwwnz.medium.com/react-portals-and-event-bubbling-8df3e35ca3f1

stshort avatar Feb 24 '22 23:02 stshort

Seeing the same thing.. we're using this via https://github.com/DimitryDushkin/sliding-pane and the tab key cant navigate around a form in the modal (slideout).. any closer to getting this fixed?

phayman avatar Feb 17 '23 08:02 phayman

@phayman Probably this library handles key events on their own, so maybe they are preventing the event to propagate (?)

diasbruno avatar Feb 17 '23 14:02 diasbruno

@diasbruno Here is an example showing how tabs work outside a modal but not inside a modal... hopefully I'm just missing something simple...

https://playcode.io/1238653

phayman avatar Feb 27 '23 13:02 phayman

Looking at the changeset from @4lejandrito .. the fix is adding the following to the CSS..

.ReactModalPortal{ position: absolute; height: 1px; width: 1px; }

See version with the above CSS fixes it https://playcode.io/1238653?v=2

vs without this css https://playcode.io/1238653

phayman avatar Mar 06 '23 14:03 phayman

@phayman actually this is currect. If you have something that has no width or height, it will be considered a "non-tababble element".

If seem that you ReactModalPortal element (the one that has the actual modal) is not getting the proper position and dimensions. You can fix it using css.

diasbruno avatar Mar 06 '23 21:03 diasbruno