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

Blocks is inserting the wrong text in the wrong node #104

Closed
Kanaye opened this issue Oct 17, 2015 · 5 comments · Fixed by #106
Closed

Blocks is inserting the wrong text in the wrong node #104

Kanaye opened this issue Oct 17, 2015 · 5 comments · Fixed by #106
Labels

Comments

@Kanaye
Copy link
Collaborator

Kanaye commented Oct 17, 2015

Blocks is updating inserting into the wrong text-node in an each data-query in the reset()- function of array-observables, when these get reset the first time.
To be clear: http://jsfiddle.net/bpkvse3r/1/ click the reset button at least 3 times.
Will look into this this tomorrow.

[edit:]
Correcting myself: The update is on the right node. The initial insert is on the wrong one.

@Kanaye Kanaye changed the title Blocks is updating the wrong text node Blocks is inserting the wrong text in the wrong node Oct 18, 2015
@Kanaye
Copy link
Collaborator Author

Kanaye commented Oct 18, 2015

Short update:
This one is more complicated then I thought.
Found the some reasons for this behaivor but there are more cases (e.g. this) where similar stuff occurs.
Could take a bit longer untill I can fix this in a clean way.

@astoilkov astoilkov added the bug label Oct 19, 2015
@astoilkov
Copy link
Owner

Let me guess. This is caused from the VirtualElement.sync method which can't find the correct order in which to sync the elements?

@Kanaye
Copy link
Collaborator Author

Kanaye commented Oct 22, 2015

The main reasons is that observables are inserting a comment when rendered with an expression.
So this:

some text
{{myExpression}}

will get rendered to something like:

some text
<!--5:blocks-->
observable content

That are 3 nodes.
We are skipping the first node if it's a coment-node.
But here we are inserting into the first node (as seen in the first fiddle) as its not a comment node.
In the second jsfiddle we are inserting all nodes again but only removing the first one.
So we are having a problem with the number of nodes a rendered expression can have.

Unfortunatly I've been busy and felt a little bit sick the last days and didn't came around to work on this.
Will look into this again later today.

@astoilkov
Copy link
Owner

Yeah. I have seen this. The main challenge is to find out how many elements to skip and which to update. There should be a mechanism which detects what type of expression is the element.

@Kanaye
Copy link
Collaborator Author

Kanaye commented Oct 23, 2015

Okay. I have a solution or at least some parts of it.
An Expression is getting it's node-length when getting rendered first time with Expression.Html and storing it, so we can remove all nodes that are related to that expression.
I added an Expression.NodeWise rendering type which will result in an array with the relative index of the node that should have that value and the rendered value.
Example:
original expression:

test:{{someObservable}}|some static

Rendered with Expression.Html:

test:
<!--5:blocks-->
test-observable|some static

will result in:

['test:', 
  null, /* maybe the comment content here as it's (most likely) an other observable in case of Observable([]).reset() */,
'test-observable|some static']

with Expression.NodeWise.

[edit:]
Forget the part with the comments content as it's not related to the dom-comment.
It's been (too) late at night as I wrote that.
[/edit]

I have to speed up some stuff and some cacheing mechanisms aren't working currently.
I also have to work around some edge cases e.g. rendering an expression where a "normal" variable is replaced in second rendering with an observable.

blocks.query({
  test: 'normal string'
});

blocks.query({ // In this case I would not have the right node value
  test: blocks.observable('and now I have a problem')
});

And I may have some other cases where the new rendering fails.

[edit: typos and brought some words in the right order]

Kanaye pushed a commit to Kanaye/jsblocks that referenced this issue Oct 24, 2015
Added an rendering type Expression.NodeWise that will return an array with
the values positioned in the order of the node when passed to
Expression.GetValue.

Expressios objects now have a property nodeLength and chunks of
expressions now have an array nodePositions containing the the position of
the nodes the expression updates.

Updated VirtualElement.syncChildren to delete all nodes related to an
expression and to update all nodes of an expression with a changed value.
astoilkov added a commit that referenced this issue Oct 28, 2015
Updating/Insetring of wrong nodes fixed. Closes #104
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 a pull request may close this issue.

2 participants