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

Better re-rendering on quick-edit #2332

Merged
merged 5 commits into from
Jan 24, 2025
Merged

Better re-rendering on quick-edit #2332

merged 5 commits into from
Jan 24, 2025

Conversation

longhotsummer
Copy link
Contributor

@longhotsummer longhotsummer commented Jan 24, 2025

  • This saves us from re-rendering an entire document when quick editing just a portion.
  • renderCoverpage uses async

In the common case of a quick-edit without an eid change, the quick-edited element is replaced.

In the other case where the eid changes or for some reason another portion of the tree changes, then the target of the mutation (If chp_1__sec_1 is quick edited, then the mutation target will be chp_1) is re-rendered.

This also fixes a bug where changing the eid and then deleting the portion would nuke the whole document.

https://www.loom.com/share/a64501a5e30d4118a3f12b3bf90d80d9

this saves us from re-rendering an entire document when quick editing
just a portion

if chp_1__sec_1 is quick edited, then the mutation target will be chp_1
and so all of chp_1 is re-rendered.

We could be smarter and only re-render chp_1__sec_1, but it gets more
complicated. We would then have to detect if the mutation impacts other
siblings (eg. the section was split into two) or if it was deleted.
Copy link

github-actions bot commented Jan 24, 2025

Test Results

569 tests  ±0   569 ✅ ±0   4m 36s ⏱️ +12s
 60 suites ±0     0 💤 ±0 
 60 files   ±0     0 ❌ ±0 

Results for commit aeaeb91. ± Comparison against base commit 5a7bf0f.

♻️ This comment has been updated with latest results.

this lets us handle the common case of a quick-edited element being
changed (with the eid not changing), meaning we can re-render just that
element
@longhotsummer
Copy link
Contributor Author

I've made this smarter to handle the common case of editing just the quick edited portion.

@longhotsummer longhotsummer marked this pull request as ready for review January 24, 2025 10:35
@longhotsummer
Copy link
Contributor Author

Fixes #2329

Copy link
Contributor

@goose-life goose-life left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

@longhotsummer longhotsummer merged commit a807b67 into master Jan 24, 2025
5 checks passed
@longhotsummer longhotsummer deleted the quick-edit branch January 24, 2025 12:50
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