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

Adding and removing CodeMirror instance on-demand, i.e. depending on … #484

Merged
merged 3 commits into from
Sep 26, 2020

Conversation

luis-mueller
Copy link
Contributor

@luis-mueller luis-mueller commented Sep 24, 2020

…the visibility of a cell - #291.

Hi all,

I took on the enhancement in #291 by only creating the CodeMirror instance if a cell is hidden (folded) and deleting the instance upon folding/hiding the cell.

I introduced a new boolean called cm_enabled that is basically !is_hidden, but allows to later add more conditions (e.g. "is cell visible on screen" - that was probably meant by viewport, right?).

The general idea is then to, instead of as before running the effect hook that set up the CodeMirror only with initial values, re-run the effect hook everytime cm_enabled changes and to wrap everything else that requires the instance into an IF that checks for cm_enabled.

This is my first contribution to this project and I hope you like it - if you have any suggestions or concerns, I'd be happy to discuss.

If this first step is done, I'd be happy to also take on the viewport-idea.

@fonsp
Copy link
Owner

fonsp commented Sep 26, 2020

Strangely I am measuring no performance benefit... I tested on:
https://github.com/fonsp/disorganised-mess/blob/master/big.jl

It does however fix a codemirror glitch when unfolding a cell, so I'm happy 🙃

@fonsp fonsp merged commit d208e05 into fonsp:master Sep 26, 2020
@fonsp
Copy link
Owner

fonsp commented Sep 26, 2020

Thank you! 🌈

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.

None yet

3 participants