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

* [jsfm] fix bug: event will not be registered to document if the "ap… #1221

Closed
wants to merge 1 commit into from

Conversation

alext1981
Copy link
Contributor

reproduce: weex/examples/component/scroller-demo.js

reproduce method: pull up the list at the end,the loading component will not be hidden, because the loading event does not be invoked

root cause: when creating the virtual DOM tree, if the root container's "append" attribute is "tree", the whole page will be create first and then be added to the document, which means that the docId will be assigned to the root container at last, while all of its children nodes can not get the docId when they are being created. A node couldn't be added to the "nodemap" of document if it do not have the docId. If an event is passed from native, the current document cannot find the proper component to handle it, because its "nodemap" is empty.

function compileNativeComponent(vm, template, dest, type) {
.......
const treeMode = template.append === 'tree'
  const app = vm._app || {}
  if (app.lastSignal !== -1 && !treeMode) {
    console.debug('[JS Framework] compile to append single node for', element)
    app.lastSignal = attachTarget(vm, element, dest)
  }
  if (app.lastSignal !== -1) {
    compileChildren(vm, template, element)
  }
  if (app.lastSignal !== -1 && treeMode) {
    console.debug('[JS Framework] compile to append whole tree for', element)
    app.lastSignal = attachTarget(vm, element, dest)
  }
}

…pend" attribute of the root container is "tree"
@CLAassistant
Copy link

CLAassistant commented Sep 13, 2016

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


aoxiao.tjb seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.

@Hanks10100
Copy link
Member

Hanks10100 commented Sep 14, 2016

@alext1981 Thanks for solving this problem. We also found this bug in #1148 , and fixed it in #1173 .

More tips for contribute weex:

  • You should sign our Contributor License Agreement first.
  • This bug is in js framework, merge your code to jsfm-feature-{version} is more appropriate.

Welcome to send PRs or open issues. Wish you could pay attention on weex continuously.

@Hanks10100 Hanks10100 self-assigned this Sep 14, 2016
@@ -84,7 +84,8 @@ function appendBody (doc, node, before) {
if (node.role === 'body') {
node.docId = doc.id
node.ownerDocument = doc
node.parentNode = documentElement
linkParent(node, documentElement)
delete doc.nodeMap[node.nodeId]
Copy link
Member

Choose a reason for hiding this comment

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

We ignored to remove the node from nodeMap, you can send PR to solve this.

@Jinjiang Jinjiang closed this Sep 14, 2016
@Jinjiang Jinjiang deleted the jsfm-bugfix-vdom branch September 27, 2016 18:11
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.

4 participants