search

Tradeshift / elements

Tradeshift Elements - Reusable Tradeshift UI Components as Web Components https://tradeshift.github.io/elements
Other
15 stars 17 forks source link

Add onOpen on the ts-aside #155

Closed CarloAl closed 5 years ago

CarloAl commented 5 years ago

We have already defined in constants.js the onOpen and onOpened events, but they are never used in aside.js (while the onClose events are actually triggered). Can we add the same for onOpen?

Kian-Esmaeilpour commented 5 years ago

I cleaned up those in this pr https://github.com/Tradeshift/elements/pull/154. For the onOpen event I don't think it make sense to have it since developers know when they are actually adding the data-visible attribute. But for the onOpened(opened after that PR) maybe we can add the event if we have a good use case for it. Did you encountered a situation that you needed that event?

CarloAl commented 5 years ago

In my case I have an aside that is used to select the visible columns in a table. Every time I open the aside I need to refresh the updated selected columns and because of the way our code is structured I don't have scope of the columns where I open the aside. I could use custom events but I guess it is pretty standard to have an onpen event in this case.