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

Add component afterRender callback #2303

Closed
wants to merge 3 commits into from

Conversation

mbest
Copy link
Member

@mbest mbest commented Oct 6, 2017

See discussion at #1533. We can discuss this specific implementation here, though.

@brianmhunt
Copy link
Member

This implementation will be completely different in tko, but I think the API surface is perfectly replicable.

Is the intention to have afterRender passed alongside params? Or have afterRender in the viewModel? Or in the ko.components.register? Or all?

childBindingContext = bindingContext['createChildContext'](componentViewModel, /* dataItemAlias */ undefined, function(ctx) {
ctx['$component'] = componentViewModel;
ctx['$componentTemplateNodes'] = originalChildNodes;
ctx._componentDisplayDeferred = displayedDeferred;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should make this a Symbol (see #2296)

@mbest
Copy link
Member Author

mbest commented Oct 6, 2017

It's just a method in the viewmodel for now. I'm not attached to calling it afterRender either. Perhaps we can use this opportunity to define a reserved set of method names. Examples: ng... (Angular), handle... (JET), ...Callback (Web Components).

@chrisknoll
Copy link

chrisknoll commented Oct 6, 2017

If it's another component parameter that sits along side 'param' we can do things like warn about invalid property assignment (I think the only valid things to the component binding is name and params). Making it a function int he view model means you have to do things like reserve function names. I know that's the pattern we have today with init, update, dispose, etc...but couldn't we consider making a dedicated, optional param to store the callback function that should be invoked after rendering?

ie:

<my-component params="param list" afterRender="theFunction"/>

and we can warn about this sort of typo:

<my-component params="param list" afterRendered="theFunction"/>
<!-- afterRendered not a valid attribute of component binding -->

Also, you don't need to be attached to a function name now, the consumer of the component can decide what they want to call their afterRender handler.

@mbest
Copy link
Member Author

mbest commented Oct 9, 2017

@chrisknoll I agree that having access to the callback in the binding is useful, but it seems that most people want to have access from within the component rather than from outside. We could have both, though, like Oracle JET does.

@chrisknoll
Copy link

I see now. Up to this point I thought the objective was so that external parties could do something after the component rendered, but I see now that it's meant to allow the component to do something for itself once it is rendered (which could be to make a callback on its own to external observers). Thanks for the clarification.

@codymullins
Copy link

Really to me this callback seems like it will be the place to handle most initialization logic. "afterRender" is the "when" it's fired but it probably wouldn't hurt to call this the init like angular $onInit, right?

@IanYates
Copy link

Having available both to the component, and to its host/parent, sounds like a good approach. I've got parts of my large codebase where I've had to do one or the other on occasion. Do you think you'll go that way with this PR?

🎉
I hacked some afterRender for components into KO enough for my needs but will happily switch to this when it's released.

@mbest
Copy link
Member Author

mbest commented Oct 16, 2017

I've created a separate pull request that adds an afterRender callback for any binding that updates and binds its descendant elements (including component): #2310. The "afterRender" defined here is different in that it waits for all child components to finish as well. I think we should call this something different to distinguish it, maybe afterComplete. Any other ideas?

@mbest
Copy link
Member Author

mbest commented Nov 13, 2017

I've been working on re-implementing this based on the work in #2310. The implementation will be more generic, and you'll be able to attach an event handler from anywhere. There will still be an automatic subscription if the component viewmodel contains a specific function. I'm thinking to call it onDescendentsComplete rather than afterRender.

@mbest
Copy link
Member Author

mbest commented Nov 14, 2017

Replaced by #2319

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants