prepareForActivationEvents() should always be called again after component re-enters document

[rayshan]

• _preparedForActivationEvents should be made public and renamed to hasPreparedForActivationEvents as it's used in EventManager and can be used in other components due to currently prepareForActivationEvents() is not called again after a component exits document, e.g. via use of Substitution https://github.com/montagejs/montage/blob/8c56ff4d51f73814295572a2f2e5a14960e4b693/ui/component.js#L1621
• Implement prepareForActivationEventsIfNeeded() and replace use here: https://github.com/montagejs/montage/blob/3dc364dc2eb067daecabe640b8611a3a38e260df/core/event/event-manager.js#L2292
• _preparedForActivationEvents is missing docs

Discussed w/ @thibaultzanini & @pchaussalet

After further discussion w/ whole team: prepareForActivationEvents() should always be called again after component re-enters document; _preparedForActivationEvents should be removed.

Final decision: rename to hasPreparedForActivationEvents, it should be reset after exitDocument so prepareForActivationEvents can be called again after enterDocument

[marchant]

Wondering if prepareForInteraction might not be simpler/straightforward. agree with public

On Feb 5, 2015, at 18:44, Ray Shan notifications@github.com wrote:

_preparedForActivationEvents should be made public and renamed to hasPreparedForActivationEvents as it's used in EventManager and can be used in other components due to currently prepareForActivationEvents() is not called again after a component exists document, e.g. via use of Substitution https://github.com/montagejs/montage/blob/8c56ff4d51f73814295572a2f2e5a14960e4b693/ui/component.js#L1621 Implement prepareForActivationEventsIfNeeded() and replace use here: https://github.com/montagejs/montage/blob/3dc364dc2eb067daecabe640b8611a3a38e260df/core/event/event-manager.js#L2292 _preparedForActivationEvents is missing docs Discussed w/ @thibaultzanini & @pchaussalet

— Reply to this email directly or view it on GitHub.

[lordbron]

I'm not sure if this is a good place to bring this up, but forgot to mention it in our meeting.

If we could fire an event or set a flag that we can bind too during this step, it would be useful to use that to let a component know when all the child components are loaded and ready. Right now, you have k ow way of knowing.

Thanks, Tom

Sent from my iPhone

On Feb 5, 2015, at 10:19 PM, Benoit Marchant notifications@github.com wrote:

Wondering if prepareForInteraction might not be simpler/straightforward. agree with public

On Feb 5, 2015, at 18:44, Ray Shan notifications@github.com wrote:

_preparedForActivationEvents should be made public and renamed to hasPreparedForActivationEvents as it's used in EventManager and can be used in other components due to currently prepareForActivationEvents() is not called again after a component exists document, e.g. via use of Substitution https://github.com/montagejs/montage/blob/8c56ff4d51f73814295572a2f2e5a14960e4b693/ui/component.js#L1621 Implement prepareForActivationEventsIfNeeded() and replace use here: https://github.com/montagejs/montage/blob/3dc364dc2eb067daecabe640b8611a3a38e260df/core/event/event-manager.js#L2292 _preparedForActivationEvents is missing docs Discussed w/ @thibaultzanini & @pchaussalet

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub.

[marchant]

That's not what this method does, it's telling a component it's about to receive mousedown, touchstart or keydown for the first time

On Feb 5, 2015, at 20:44, lordbron notifications@github.com wrote:

I'm not sure if this is a good place to bring this up, but forgot to mention it in our meeting.

If we could fire an event or set a flag that we can bind too during this step, it would be useful to use that to let a component know when all the child components are loaded and ready. Right now, you have k ow way of knowing.

Thanks, Tom

Sent from my iPhone

On Feb 5, 2015, at 10:19 PM, Benoit Marchant notifications@github.com wrote:

Wondering if prepareForInteraction might not be simpler/straightforward. agree with public

On Feb 5, 2015, at 18:44, Ray Shan notifications@github.com wrote:

_preparedForActivationEvents should be made public and renamed to hasPreparedForActivationEvents as it's used in EventManager and can be used in other components due to currently prepareForActivationEvents() is not called again after a component exists document, e.g. via use of Substitution https://github.com/montagejs/montage/blob/8c56ff4d51f73814295572a2f2e5a14960e4b693/ui/component.js#L1621 Implement prepareForActivationEventsIfNeeded() and replace use here: https://github.com/montagejs/montage/blob/3dc364dc2eb067daecabe640b8611a3a38e260df/core/event/event-manager.js#L2292 _preparedForActivationEvents is missing docs Discussed w/ @thibaultzanini & @pchaussalet

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub.

[rayshan]

If we were to simplify I recommend prepareForAction to be consistent for the action events we emit.

Also now thinking twice about # 2, all prepareForActivationEventsIfNeeded() would do is to check the flag. If we make the flag public, maybe it's overkill to offer such a small convenience.

[dharcourt]

What would happen if a developer adds listeners in the prepareForAction() of a component (typical use case), then the component is removed from the DOM and added back again? With the proposed behavior of calling prepareForAction() multiple times will multiple listeners be added unless developers do additional work? If so, is this avoidable? If not, how can make things as simple as possible for developers (e.g. have a cleanup method equivalent to prepareForAction(), or document the necessity of cleaning listeners up in exitDocument())?

[dharcourt]

By the way, the newly public property should be isPreparedForActivationEvents (or isPreparedForAction), not hasPreparedForActivationEvent (or hasPreparedForAction).

The "prepared" here is an adjective representing a boolean property, so "isPrepared" represents a query of that property. "hasPrepared", on the other hand, would be the present perfect or the verb "prepare" and so would not match the "is"/"has" pattern of boolean property names. While "has" can be used for boolean property names, it is used to denote ownership as in "hasElements", not to create a present perfect as in "hasPrepared".

[dharcourt]

Answering my own questions from 2 comments above: Multiple listener adding calls will be made, but because they'll be for the same listener and event, multiple listeners will not be added. The multiple addListener() calls would be generally unavoidable within the scope of this issue (smarter listeners would allow for this but that's outside this issue's scope). Making the isPreparedForActivationEvents property public would provide what most developers will need to do things right, and we should suggest the following pattern in the documentation for preparedForActivationEvents:

preparedForActivationEvents() {
    // Set up listeners, bindings, etc...
}

exitDocument() {
    if (this.isPreparedForActivationEvents) {
        // Clean up listeners etc....
    }
}
'''

[marchant]

We may want to improve the API by passing the event, after all, if the event coming in is a touch, why add mouse event listeners? Also, as much as possible we should have event composers do this themselves.

On May 27, 2015, at 2:53 PM, dharcourt notifications@github.com wrote:

Answering my own questions from 2 comments above: Multiple listener adding calls will be made, but because they'll be for the same listener and event, multiple listeners will not be added. The multiple addListener() calls would be generally unavoidable within the scope of this issue (smarter listeners would allow for this but that's outside this issue's scope). Making the isPreparedForActivationEvents property public would provide what most developers will need to do things right, and we should suggest the following pattern in the documentation for preparedForActivationEvents:

preparedForActivationEvents() { // Set up listeners, bindings, etc... }

exitDocument() { if (this.isPreparedForActivationEvents) { // Clean up listeners etc.... } } '''

— Reply to this email directly or view it on GitHub https://github.com/montagejs/montage/issues/1521#issuecomment-106089210.

http://montagestudio.com/ montagestudio.com http://montagestudio.com/
Benoit Marchant mailto:benoit@montagestudio.com CEO & Founder 1290 Oakmead Parkway, Ste 111 Sunnyvale, CA 94085 (1) 408.621.4874 mobile tel:+1-408-621-4874 Profile http://www.linkedin.com/in/benoitmarchant | @benoitmarchant http://twitter.com/benoitmarchant