-
-
Notifications
You must be signed in to change notification settings - Fork 191
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
Add igv component #537
Add igv component #537
Conversation
When you add docs for this, can you please also add docs for dash-ngl at the same time, since it never got documented over at https://dash.plotly.com/dash-bio |
src/lib/fragments/Igv.react.js
Outdated
*/ | ||
export default class Igv extends Component { | ||
componentDidMount() { | ||
var igvContainer = document.getElementById(this.props.id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't use getElementById
- use a ref
instead. For an example, see https://github.com/plotly/dash-core-components/pull/604/files where this change was made to dcc.Graph
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed in 0e650d0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beautiful! 💃 once tests pass (looks like just a minor lint issue)
Closes plotly/dash-core#250
About
Description of changes
This PR is a continuation of the draft PR created by @shday here. This PR includes the following additions to the original PR:
igv.react.js
to enable async loading of the component and greatly reduce bundle size.app.get_asset_url
to pass a URL directly to the IGV browser.igv
package from experimentalreact
fork to (nowreact
) compatiblemaster
fork.