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

[Aside] Aside.onclose() should cancel on return false #255

Closed
wiredearp opened this issue May 4, 2017 · 1 comment
Closed

[Aside] Aside.onclose() should cancel on return false #255

wiredearp opened this issue May 4, 2017 · 1 comment
Milestone

Comments

@wiredearp
Copy link
Contributor

wiredearp commented May 4, 2017

@sampi @zdlm @wejendorp

And the same goes for Notificatons and Dialogs and the SideBar in mobile breakpoint and I guess the DatePicker and the standard select picker and what not: It is a pattern that the Flux/Redux store can be the single source of truth for UI state and thus the automatic closing of components can be considered an illegal mutation of state; this would happen onclick the closing X, onclick the aside cover and onkeypress ESCAPE.

This should already be possible according to https://github.com/Tradeshift/tradeshift-ui/blob/master/src/runtime/js/ts.ui/core/core-gui%40tradeshift.com/spirits/asides/ts.ui.SideShowSpirit.js#L633 and https://github.com/Tradeshift/tradeshift-ui/blob/master/src/runtime/js/ts.ui/core/core-gui%40tradeshift.com/spirits/asides/ts.ui.AsideSpirit.js#L393, but apparently this doesn't really work so great, because the general UI (and the cover in particular) is not aware of the return value.

@wejendorp
Copy link
Contributor

Nice summary, I have no more to add. Let me know when there's anything to test.

@zdlm zdlm added this to the 7.3.0 milestone May 9, 2017
@wiredearp wiredearp changed the title Aside.onclose() should cancel on return false [Aside] Aside.onclose() should cancel on return false May 10, 2017
marinalecuts pushed a commit to marinalecuts/tradeshift-ui that referenced this issue Jun 13, 2019
* [Inbox] Fixed hack refresh feature

* [Header] Removed unused PropType

* [preload] Remove unused `<link rel=“preload” as=“document”>`

* [Root] Removed extra layer of nesting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants