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

download: use the node.shell.inView property #1681

Merged
merged 1 commit into from
Aug 20, 2023

Conversation

joverlee521
Copy link
Contributor

@joverlee521 joverlee521 commented Jul 27, 2023

Description of proposed changes

The node.inView property is a temporary hold for the node's inView property to be able to zoom in on specific branches before the D3 PhyloTree has been constructed.

Once the D3 PhyloTree has been constructed, the inView property is available through node.shell.inView. This is the property that gets updated by applyInViewNodesToTree whenever the user interacts with any zooming functionality on the tree. The temporary node.inView property will never be updated and thus only represents the initial state of the node when the tree was first loaded.

Therefore, any downstream uses of the inView property should be using the node.shell.inView property.

Related issue(s)

Fixes #1411

Testing

The `node.inView` property is a temporary hold for the node's inView
property to be able to zoom in on specific branches before the
D3 PhyloTree has been constructed.

Once the D3 PhyloTree has been constructed, the inView property is
available through `node.shell.inView`. This is the property that gets
updated by `applyInViewNodesToTree` whenever the user interacts with
any zooming functionality on the tree. The temporary `node.inView`
property will never be updated and thus only represents the initial
state of the node when the tree was first loaded.

Therefore, any downstream uses of the inView property should be using
the `node.shell.inView` property.
@joverlee521 joverlee521 requested a review from a team July 27, 2023 20:50
@nextstrain-bot nextstrain-bot temporarily deployed to auspice-fix-blank-metad-l69n9u July 27, 2023 20:50 Inactive
@jameshadfield jameshadfield self-requested a review July 27, 2023 22:15
Copy link
Member

@jameshadfield jameshadfield left a comment

Choose a reason for hiding this comment

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

Thanks @joverlee521 - was able to recreate the error from #1411 (comment) & confirm this fixes it.

@jameshadfield jameshadfield merged commit 337c28e into master Aug 20, 2023
@jameshadfield jameshadfield deleted the fix-blank-metadata branch August 20, 2023 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Blank metadata download
3 participants