naturalcrit / homebrewery

Create authentic looking D&D homebrews using only markdown
https://homebrewery.naturalcrit.com
MIT License
1.1k stars 327 forks source link

Getting the current Editor Page for page rendering happens out of sync #3317

Open calculuschild opened 9 months ago

calculuschild commented 9 months ago

See #3000 which fixes the symptom but not the root cause.

The error is caused by the addition of this line:

https://github.com/naturalcrit/homebrewery/blob/4feeaee7e779391bbfe4de61c786864b408ddac9/client/homebrew/brewRenderer/brewRenderer.jsx#L155

This line is to make sure when rendering pages, we always render the currently-edited page first (which may not be in view on the brewRenderer preview). This ensures that any changes that have cross-page effects (e.g. variables or links) have their changes propagated correctly to the pages in view.

Unfortunately, updating the "current editor page" happens slightly out of sync here:

1) Codemirror sees a text change in codeEditor.jsx 2) Sends this new text up to handleTextChange() in editPage.jsx (or newPage or homePage) 3) handleTextChange() requests from editor.jsx the currently edited page based on cursor position 3) editPage.jsx propagates new text down to editor.jsx and brewRenderer.jsx.

As you can see, editor.jsx is being asked to calculate the latest cursor position before it it has received the latest text.

https://github.com/naturalcrit/homebrewery/blob/4feeaee7e779391bbfe4de61c786864b408ddac9/client/homebrew/pages/editPage/editPage.jsx#L116

This means, deleting the last \page (or part of it) will cause currentEditedPage to point beyond the total page count, thus attempting to render (and sanitize) an empty value.

It might be worth moving getCurrentPage() to come directly from Codemirror in codeEditor.jsx instead , or just add the page number as a second parameter to handleTextChange(). Not sure the best way to handle this, but I don't want this to be lost.

dbolack-ab commented 2 days ago

I believe this has been fixed by somewhat recent code as part of the spread view.

5e-Cleric commented 1 day ago

The issue still stands, showing only in some screens, not sure why. I get the issue in one of my screens, not the other.