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

Contain all rendering related tasks in DOMView.render() #13871

Open
wants to merge 1 commit into
base: branch-3.5
Choose a base branch
from

Conversation

mattpap
Copy link
Contributor

@mattpap mattpap commented May 6, 2024

Currently rendering is spread in multiple places, including initialize(), connect_signals() and render(). When re-rendering this.el (the root element of a component) never gets recreated. It's "emptied" instead, but that doesn't really clear all the associated state with that element, which can cause issues when calling render() multiple times.

This PR moves all rendering related tasks to render() method, including creation of this.el. If re-rendering, then render() will create a new DOM element and then use replaceWith to efficiently replace it in the existing DOM hierarchy. This makes re-rendering much more predictable. Re-rendering, especially unnecessary re-rendering, is not advised, but sometimes necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant