downshift version: 3.3.1 (I believe it affects all versions)node version:npm (or yarn) version:Relevant code or config
What you did:
I unmounted the component
What happened:
The accessibility status message remained in the DOM
Problem description:
The accessibility status message remains in the DOM after the component has been unmounted
Suggested solution:
Remove the accessibility status message on unmount
We could add another helper method in the set-a11y-status.js that cleans the element and then use it in comonentWillUnmount for cleanup.
Do you want to create a PR with the fix, test(s) for both useSelect and Downshift and add yourself as contributor?
I'm having a related issue
<div
aria-live="polite"
aria-relevant="additions text"
id="a11y-status-message"
role="status"
style="border: 0px; height: 1px; margin: -1px; overflow: hidden; padding: 0px; position: absolute; width: 1px;"
>
Text
</div>
It only happens from time to time, and causes flakey tests for us.
I will try to figure out a way to fix this.
As far as the flakey tests are concerned @beckerei, are you sure you are using fake timers then flushing them out correctly every time?
On the fact that the status in not cleaned up on unmount, can you provide a sandbox where this is happening? Since we are using a debounce internal implementation based on setTimeout then the cleanup should perform after the timeout of it last being called expired. @MarkFalconbridge
As far as the flakey tests are concerned @beckerei, are you sure you are using fake timers then flushing them out correctly every time?
I wasn't using fakeTimers, but started to use them after I looked through the code and noticed how the cleanup works.
I scanned through the docs and could not find anything there, so it might be worth adding a little section about testing?
I'm not sure if @MarkFalconbridge has problems in his app or his tests as well.
However currently the actual node always remains in the DOM, just the message is cleared.
Here is a sandbox to illustrate that the a11y-status-message div remains in the DOM after Downshift has been unmounted. It's a modified version of the typeahead example. To see the issue, trigger the dropdown using the toggle button in the Downshift component, this should add the a11y-status-message div. Then close the dropdown and then click the button I added to remove Downshift. After this happens I would expect Downshift to tidy up the a11y-status-message div it added but it doesn't.
It might be a little tricky to just remove the div. A few things need to be answered before we could tackle this.
What happens when you have more than one instance of downshift?
Should each of these maintain their own status div? What a11y impact has this?
Should the div be removed once the last instance of downshift is removed?
I still offer to help here, even though my first iteration was not what you were looking for 馃憤
If we remove the div at dismount then it should still work, as in set-a11y-status if the div is not found then it will create a new one.
@beckerei if now that the issue is clear (remove the div at unmount, not the text) can you create another PR? I will review it.
Thanks a lot guys!
I made up my mind with this. Not removing it from our code, will let the client do it, if he wants to remove the element. I don't think it's a big issue and it can be handled externally with ease, we should not add more code to the package.
Most helpful comment
I'm having a related issue
It only happens from time to time, and causes flakey tests for us.
I will try to figure out a way to fix this.