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

Use Element: replaceChildren() method to remove all child nodes. #1359

Closed
wants to merge 5 commits into from

Conversation

beru
Copy link

@beru beru commented Sep 19, 2024

This PR replaces innerHTML = '' with replaceChildren() to remove all child nodes.

https://stackoverflow.com/a/65413839
https://developer.mozilla.org/en-US/docs/Web/API/Element/replaceChildren

view.FindSidebar.deactivate is also updated in the hope of resolving the problem reported in #1358. Although the change doesn't solve the problem, closing the sidebar gets faster.

recording

efficientdet-d3.onnx file is used.

lifespan

@lutzroeder
Copy link
Owner

lutzroeder commented Sep 19, 2024

replaceChildren is more recent and requires Chrome 86.

Is the improvement from calling in deactivate and would iterating on lastChild with removeChild work?

@beru
Copy link
Author

beru commented Sep 19, 2024

replaceChildren is more recent and requires Chrome 86.

Added a polyfill function for that. e8b72a5

Is the improvement from calling in deactivate and would iterating on lastChild with removeChild work?

Yes, I've confirmed that doing so still works.

I've found out that since I've added this._content.replaceChildren(); line in view.FindSidebar.deactivate, I need to add view.FindSidebar.activate method and call this._update(); in there. And the code in view.FindSidebar.focus needs to be moved to view.FindSidebar.activate. I hope this change is correct. Please examine it at your side as well.


I've noticed that when I double-click Connections/Weights list items on FIND sidebar or click connections on the graph view, CONNECTION PROPERTIES/TENSOR PROPERTIES sidebar is opened. After closing the sidebar, FIND sidebar reappers. But this is not the case with Nodes list items. After closing NODE PROPERTIES sidebar, FIND sidebar doesn't reappear. I thought this is a bit incosistent but I'm not sure what is a proper behavior here.

lutzroeder added a commit that referenced this pull request Sep 20, 2024
@lutzroeder lutzroeder closed this Sep 20, 2024
@beru beru deleted the replaceChildren branch September 20, 2024 05:00
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.

2 participants