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

Single component redraw fix #743

Closed
wants to merge 5 commits into from
Closed

Single component redraw fix #743

wants to merge 5 commits into from

Conversation

dhinesh03
Copy link

@dhinesh03 dhinesh03 commented Jul 28, 2015

Applied single component redraw patch on the current master.

Applied @jonahx ’s single component redraw patch on the current master.
for (var i = 0; i < controllers.length; i++) {
var controller = controllers[i];
if (isFunction(controller.redrawSelf)) {
controller.redrawSelf = (function(ctrl,view) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@dhinesh03
Copy link
Author

@Naddiseo Thanks for pointing it out.

var views = [], controllers = [];
data = markViews(data, cached, views, controllers);
if (!data.tag && controllers.length) throw new Error("Component template must return a virtual element, not an array, string, etc.");
if (!data.tag && controllers.length) throw new Error("Component template must return a virtual element, not an array, string, etc.");
Copy link
Contributor

Choose a reason for hiding this comment

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

trailing whitespace

@Naddiseo
Copy link
Contributor

How does redrawSelf get set initially?

Is there a way to merge the for-loop with another for-loop called indirectly earlier on in the function?

Removed trailing whitespace and added space after comma
@dhinesh03
Copy link
Author

@Naddiseo

  1. We should set redrawSelf as empty function where ever needed.
redrawSelf = function () {
        };

Refer @jonahx 's codepen for more details.
2) I've tried, but the cached object will be overwritten on the following line

cached = reconstructCached(data, attrs, children, node, namespace, views, controllers); 

@avesus
Copy link
Contributor

avesus commented Aug 28, 2015

@dhinesh03 please, consider adding a documentation for controller.redrawSelf(). Also, could we write a test for this?

Necessary config functions has been invoked after self-redraw.
@dhinesh03
Copy link
Author

@avesus Busy with my work, Will add doc soon.

configs - array not initialized while self redraw  issue fixed
@sebastiansandqvist
Copy link
Contributor

@dhinesh03 I'd definitely like to see something like this in a future release. What is the status of this pr?

@dead-claudia
Copy link
Member

Closing for now (cleaning up old PRs). Please open a new issue or PR if you want this to be fixed/added/implemented.

@jonahx
Copy link

jonahx commented Dec 10, 2015

Closing it and reopening it seems like it makes things messier, to me. I'm still interested in this, and based on thread comments no reason to think other people's interest has changed.

@dead-claudia
Copy link
Member

You want to run a diff for yourself? It has tons of merge conflicts, and is best just redone.

@jonahx
Copy link

jonahx commented Dec 11, 2015

I’m happy to redo it if there are any plans to merge it. If not,
nothing has changed from the original PR, so it’s probably more
accurate to close it as WON’T FIX.

On 10 Dec 2015, at 19:18, Isiah Meadows wrote:

You want to run a diff for yourself? It has tons of merge conflicts,
and is best just redone.


Reply to this email directly or view it on GitHub:
#743 (comment)

@dead-claudia
Copy link
Member

If you clean this up (trust me in that you'd have to deal with a massive
merge conflict), I can reopen it once it's updated.

On Thu, Dec 10, 2015, 21:07 Jonah notifications@github.com wrote:

I’m happy to redo it if there are any plans to merge it. If not,
nothing has changed from the original PR, so it’s probably more
accurate to close it as WON’T FIX.

On 10 Dec 2015, at 19:18, Isiah Meadows wrote:

You want to run a diff for yourself? It has tons of merge conflicts,
and is best just redone.


Reply to this email directly or view it on GitHub:
#743 (comment)


Reply to this email directly or view it on GitHub
#743 (comment).

@willycamargo
Copy link

Any plans to implement and merge it? It is a great feature for some applications.

My application has many components working at the same time, and sometimes this feature would be good.

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

7 participants