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

The ViewWrapper removes the HTML if the instance is destroyed and the visible state is kept #336

Closed
lehnerchristian opened this issue Mar 27, 2020 · 1 comment
Assignees

Comments

@lehnerchristian
Copy link

lehnerchristian commented Mar 27, 2020

ViewWrapper deletes its own node from the DOM

The ViewWrapper class removes its own HTML node a detail view is mounted in when the instance is destroyed: https://github.com/datavisyn/tdp_core/blob/develop/src/base/ViewWrapper.ts#L209

The problem is that the node is only created when the ViewWrapper is initialized (see https://github.com/datavisyn/tdp_core/blob/develop/src/base/ViewWrapper.ts#L64), which is done differently in many apps, but generally speaking the ViewWrappers are instantiated when the detail view chooser is built

Visible state is kept when destroying the instance

A second related problem is that if the view is destroyed the ViewWrappers visible state is not set to false, which leads to the bug that if the ViewWrapper instance is destroyed and the main ranking view in an application is clicked that suddenly a detail view is instantiated without choosing a view from the detail view chooser. in the following example matches && this.visible is true although visible should be false when the instance was destroyed:
see https://github.com/datavisyn/tdp_core/blob/develop/src/base/ViewWrapper.ts#L298-L300

@lehnerchristian lehnerchristian self-assigned this Mar 27, 2020
lehnerchristian pushed a commit that referenced this issue Mar 27, 2020
@lehnerchristian
Copy link
Author

we agreed that the node should be removed when the destroy method is invoked, because that is what is expected. so closing this ticket

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

No branches or pull requests

1 participant