search

Tradeshift / tradeshift-ui

The Tradeshift UI Library & Framework
https://ui.tradeshift.com
Other
33 stars 45 forks source link

[Modal] Fix broadcast issues for data-ts attributes #281

Closed tolrodco closed 6 years ago

tolrodco commented 7 years ago

Bug report

Modal does not have available the broadcast to be able to bind listeners such as using ts-closed, to indicate completion of modal transition. It appears to be only localized with the ui library and not available outside of it. Currently the ts.ui.get('#container').close(); function is the safest way to complete the action. While perfectly fine, the data-ts.open method has been the standard way to handle the transitions with the Aside for example.

Tradeshift UI version affected

7.2 -> Current

Expected Behavior

User clicks the X at the top right or uses the esc key to close the modal. A broadcast should tell any listener that the modal has completed its closing actions.

Actual Behavior

Modal closes but does not update any listener that the modal has been closed, preventing actions that would be dependent upon a successful action.

React, in particular, does not know if the modal has closed, and, assumes the modal is still open. As such, it will not update the DOM with a new view.

The current model is to use an API call to ts.ui to close the modal, then, update the state so React knows it has been closed. Once the state is updated, React can then supply another modal when called.

Steps to reproduce

Add data-ts to modal container, and, use existing methods for listeners such as (document.addEvenListener('ts-closed', action, this);. Event listener doesn't appear to ever fire or receive a broadcast for the modal.

wiredearp commented 7 years ago

@tolrodco and @romeldris, you guys can try the Modal once again on your localhost with version 8.0.4. You can change this in the config.json in Apps-Server (and then restart Apps-Server). We are not completely sure when this fix will be merged to master in V4, so please don't rely on it if your project is due for release soonish, but we would otherwise like some feedback on whether or not the operation was successful. We've added a section title "Tracking the state" over in the Modal docs on http://ui.tradeshift.com/#components/modals/.

sampi commented 6 years ago

Fixed. Closing.