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

support auto scan #18

Merged
merged 1 commit into from
Jun 12, 2015
Merged

support auto scan #18

merged 1 commit into from
Jun 12, 2015

Conversation

kaesonho
Copy link
Contributor

@kaesonho kaesonho commented Jun 4, 2015

#8

@redonkulus @lingyan

support auto scan to track links, it's not a recommend way but sometime users will need that, for example when they are using dangerouslySetInnerHTML, they cannot change <a> to <I13nAnchor>, they will need this, we will scan tags with componentDidMount by getElementsByTagName, and

  • create i13nNode
  • setup viewport detection
  • add debug dashboard
    for each scanned links

Usage

add scanLinks as props in I13nComponent,

<I13nDiv i13nModel={i13nModel} scanLinks={{enable:true, tags:['a', 'button']}}>
    // the links inside will be scanned and tracked
    // the i13n data will apply the i13nModel from I13nDiv
    <a href="/foo">foo</a>
    <button>bar</button>
</I13nDiv>

@yahoocla
Copy link

yahoocla commented Jun 4, 2015

CLA is valid!

return;
}
self._detectElement(self._i13nNode, self.enterViewportCallback, self.exitViewportCallback);
self._subComponentsViewportDetection && self._subComponentsViewportDetection();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

change some detail implementation of viewport detection, will detect subcomponents as well

@lingyan
Copy link
Member

lingyan commented Jun 8, 2015

👍

@kaesonho
Copy link
Contributor Author

waiting for @redonkulus's ship then I will rebase and merge it.


### Inherit Architecture
* We can define i13n data for each level, whenever we want to get the `i13nModel` for certain node, it traverses back to the root and merge all the `i13nModel` information in the path. Since the tree is already built and we don't need extra DOM access, it should be pretty cheap and efficient.
>>>>>>> modify for feedbacks
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like a diff issue. also was main ideas supposed to be added?

@kaesonho
Copy link
Contributor Author

updated according to the feedback

@@ -16,6 +16,16 @@ Typically, you have to manually add instrumentation code throughout your applica
* **Performant** - Tracking data (`i13nModel`) can be a plain JS object or custom function. This means you can [dynamically change tracking data](./docs/guides/integrateWithComponents.md#dynamic-i13n-model) without causing unnecessary re-renders.
* **Adaptable** - If you are using an isomorphic framework (e.g. [Fluxible](http://fluxible.io)) to build your app, you can easily [change the tracking implementation](./docs/guides/createPlugins.md) on the server and client side. For example, to track page views, you can fire an http request on server and xhr request on the client.
* **Optimizable** - We provide an option to enable viewport checking for each `I13nNode`. Which means that data will only be beaconed when the node is in the viewport. This reduces the network usage for the user and provides better tracking details.
* **Auto Scan Links** - Support [auto scan links](./docs/api/createI13nNode.md) for the cases you are not able to replace the component you are using to get it tracked, e.g., if you have dependencies or you are using `dangerouslySetInnerHTML`. We scan the tags you define on client side, track them and build nodes for them in i13n tree.

## Main Ideas
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need this section as I already added it here. I just made it part of the i13n Tree section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I cannot recall why did I add this, I guess it's a copy mistake from some old branch, already remove it :)

@redonkulus
Copy link
Contributor

squash and 👍

kaesonho added a commit that referenced this pull request Jun 12, 2015
@kaesonho kaesonho merged commit c4f6f7e into master Jun 12, 2015
@kaesonho kaesonho deleted the supportAutoScan branch June 12, 2015 21:40
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