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

data-if attribute to conditionally remove element from DOM #50

Closed
LeaVerou opened this issue Jul 5, 2016 · 1 comment
Closed

data-if attribute to conditionally remove element from DOM #50

LeaVerou opened this issue Jul 5, 2016 · 1 comment

Comments

@LeaVerou
Copy link
Member

LeaVerou commented Jul 5, 2016

No description provided.

@LeaVerou LeaVerou added this to the Public release milestone Jul 5, 2016
LeaVerou added a commit that referenced this issue Dec 8, 2016
Properties inside the removed element are still reflected in
expressions.
Also fixed bug with display of mv-bar
@LeaVerou
Copy link
Member Author

LeaVerou commented Dec 9, 2016

As discussed yesterday with @karger, documenting here for posterity:

Problem 1: If properties are inside a false if, do we save them?
Resolution: YES. This way if can be used for things like filtering and pagination without data loss.

Problem 2: Currently, multiple properties with the same name create an immutable collection. However, this might be confusing with structures like the following:

<div mv-if="...">
    <div property="foo" ></div>
</div>
<div mv-if="...">
    <div property="foo" ></div>
</div>

where the expectation is 1 property. However, in the general case, both ifs could be true, so we basically have something that could switch from a collection to a singleton every time the conditions recompute but is still saved as a collection due to the resolution above.

Implementation-wise, probably the easiest course of action is to keep this as a collection, since it's saved as one, and make the change that getData() on immutable collections returns a singleton instead of an array with one element.

LeaVerou added a commit that referenced this issue Apr 3, 2017
Basically the resolution from
#50 (comment) was
not applied
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant