manifold icon indicating copy to clipboard operation
manifold copied to clipboard

Closing an Admin Interface Listbox Contained in an Edit Modal Attempts to Close the Modal

Open Santoz2 opened this issue 5 years ago • 8 comments

In the Admin Interface, a lot of Editing happens in a modal on the side of the screen. If there is a Listbox contained in that modal, attempting to close the Listbox instead attempts to close the whole modal, which causes confusion and could potentially lead to accidental deletion of edits.

Expected Behavior

Closing an expanded Listbox should close the Listbox and place focus back on the button that opened that opened it.

Current Behavior

Closing certain Listboxes in the Admin Interface opens an unrelated popup modal, causing confusion.

Steps to Reproduce

  1. Navigate to Admin Interface (via Enter Admin Mode)
  2. Navigate to an editable project
  3. Navigate to the Layout page
  4. Edit the Calls to Action, and activate the "Add Button to Left Side" button
  5. Navigate to and activate the "type" Listbox
  6. Press the arrow down key to change the selected option
  7. Press escape to close the Listbox
  8. Notice how a new "Are you Sure" modal pops up (also note that "Are You Sure" is not announced, but this will be its own issue).
  9. Note how continually pressing escape continues to close and reopen this modal

Testing Environment

NVDA 2019.2.1 ESR Firefox 68.2.0 esr

Santoz2 avatar Nov 04 '19 23:11 Santoz2

Thanks for documenting this for us @Santoz2. I'm actually seeing four different issues when I review this:

  1. Pushing ESC with the listbox active triggers the drawer's keyup listener. I'm seeing this in Firefox and Safari, but not Chrome.
  2. The "Are you sure?" modal isn't announced.
  3. Pushing ESC in the "Are you sure?" modal triggers the drawer's keyup listener.
  4. The color of the listbox is black in Firefox. This likely can be traced to trying to set a color for the options, since FF is unique in inheriting this color from the <select>, and would otherwise appear white in this context. Screen Shot 2019-11-07 at 2 25 40 PM

dananjohnson avatar Nov 07 '19 22:11 dananjohnson

I'm going to break these up and track them as separate issues.

dananjohnson avatar Nov 07 '19 22:11 dananjohnson

No. 2 is tracked at #2430 No. 4 is tracked at #2429

dananjohnson avatar Nov 07 '19 22:11 dananjohnson

Thank you for breaking these up!

Santoz2 avatar Nov 08 '19 20:11 Santoz2

@dananjohnson you need anything from me to close this one out?

zdavis avatar Feb 12 '20 21:02 zdavis

@zdavis as far as I can tell, this issue was in mind when the Drawer component was initially built, but the attempted solution doesn't actually work. Specifically, <Drawer /> tries to use the Context API to pause keyboard events when a Dialog component is mounted. However, the the Dialog is higher up the tree than Drawer, so it can't actually be a consumer of the Drawer's context. Before jumping back into this, I wanted to check with you about how best to resolve this. It looks like the context provider would to be near the very top of the tree if it's going to be consumed by <Dialog />.

Sidenote: I started by updating to the new Context API here, since the old API is now deprecated.

dananjohnson avatar Mar 24 '20 18:03 dananjohnson

I don't really have a strong thought on this. Can you resolve it without my input, @dananjohnson, or do I need to dig into it?

zdavis avatar Sep 29 '21 15:09 zdavis

@zdavis it's been awhile, so let me take a look again. I should be able to solve it.

dananjohnson avatar Sep 29 '21 16:09 dananjohnson