Skip to content

Don't render text when the view is hidden #251583

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

Closed

Conversation

pisv
Copy link
Contributor

@pisv pisv commented Jun 16, 2025

Currently, there is a check in view.ts that avoids rendering text when the view node is not connected. This PR proposes to add another simple check that would avoid rendering text when the view node is hidden.

It would be useful for Monaco adopters such as Eclipse Theia, which have an editor lifecycle that differs from the editor lifecycle in the VS Code workbench. As such, it should make Monaco more reusable outside VS Code.

Please see eclipse-theia/theia#15500 (comment) for some background.

Thank you!

@hediet
Copy link
Member

hediet commented Jul 3, 2025

Thanks for the PR! What happens if the expression !this.domNode.domNode.offsetParent will evaluate to true at some later moment in time? Will the text be rerendered?

@pisv
Copy link
Contributor Author

pisv commented Jul 3, 2025

Thanks for the PR! What happens if the expression !this.domNode.domNode.offsetParent will evaluate to true at some later moment in time? Will the text be rerendered?

Thanks for looking! I guess the answer is probably 'no', but is not it similar to !this.domNode.domNode.isConnected? (I've quickly searched for references to isConnected and couldn't find any relevant hints for this case... Am I missing something?)

@hediet
Copy link
Member

hediet commented Jul 3, 2025

I've quickly searched for references to isConnected and couldn't find any relevant hints for this case...

Maybe its a bug there already, I don't even know what isConnected means.
Would be good to have a full understanding of how the editor behaves when the editor transitions between visible and not visible.
This check seems like a hot fix for a specific scenarios, but I'm sure it can cause other problems.

@pisv
Copy link
Contributor Author

pisv commented Jul 3, 2025

You are probably right. Let's close it then. Thank you.

@pisv pisv closed this Jul 3, 2025
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.

4 participants