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

Propagate active property to tail #176

Closed

Conversation

TimvdLippe
Copy link

This is an update of #125 to incorporate the feedback of @e111077 and to rebase onto current master. As the repository was moved after I made the original PR, I was unable to push to the original PR.

Supersedes #125
Fixes #110
Closes #77

@scbergeron
Copy link

Any update on this pull request ?

@RackhamLeNoir
Copy link

For the time being, I use a workaround which is fine in my situation. I used to bind the tail data to an iron-pages and the default was set with fallback-selection="home". So I changed all my links /foo to /foo/home. In this case I always overwrite the previous value. Hope it can help others.

@vinerz
Copy link

vinerz commented Apr 1, 2018

Almost two years later, any news about this?

@e111077
Copy link
Member

e111077 commented Apr 1, 2018

Hello, we considered this when we created app-route, but initially went against doing it largely because of thrashing the databinding system. Thus, the element is working as intended.

We have since put our elements on feature freeze in an effort to lessen our support surface for our understaffed team, and since this element is working as intended, this PR is considered a feature. I have left this PR open as an easy way for users of this element to find this fork.

@mercmobily
Copy link

When you say "working as intended"... can you please expand on it? Does the new binding system that comes with P2 make it possible to have this implemented correctly?

@e111077
Copy link
Member

e111077 commented Apr 18, 2018

Well it isn't necessarily thrashing, but it would cause the databinding system to fire for an entire subtree of active / inactive elements. App route is already computationally inefficient so we took this definicency into account when creating the element.

@mercmobily
Copy link

@e111077 ok but... What is the point of having the tail/active if they are 100% unreliable? Or, is there a recommended set of workarounds/best practices to still use it effectively?

@e111077
Copy link
Member

e111077 commented Apr 18, 2018

It should be reliable at the level of the app-route tree that was made inactive which should always be up the tree.

With this interaction model, the view handling that newly-deactivated app-route should be in charge of deactivating the view below it.

@e111077
Copy link
Member

e111077 commented Apr 18, 2018

Perhaps "active" is a bad name for this as it is a declarative representation of a "deactivate" callback

@mercmobily
Copy link

mercmobily commented May 15, 2018 via email

@TimvdLippe
Copy link
Author

This PR is very old and I am no longer pursuing this change. Therefore I am closing this PR.

@TimvdLippe TimvdLippe closed this Aug 10, 2018
@TimvdLippe TimvdLippe deleted the active-propagation branch August 10, 2018 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants