Advanced checklist fixes #50
Conversation
| description, | ||
| completed: IS_COMPLETED.test(rawTask), | ||
| createdAt: new Date(), | ||
| createdAt: new Date().toISOString(), |
There was a problem hiding this comment.
How will this work with existing notes?
There was a problem hiding this comment.
Great question! The "non-serializable values" message from Redux is more of a warning than an error I think, as this was working until now and I only discovered the message by running the plugin locally. Existing notes will still have dates as createdAt and will trigger the warning, but the comparison in line 130 in this file should still work okay since doing new Date() on a date will do nothing. We could also "migrate" the createdAt values of existing notes to strings, but I'd say it's not worth it -- if I'm understanding correctly, the only thing that lastActive comparison does for now is trigger the autofocus on the input for the last group.
There was a problem hiding this comment.
Cool, sounds good. As long as we can be fairly certain that existing notes won't break its fine. Also, nice work figuring out what was causing this, it has been broken for a while. Was it the dispatch(tasksLoaded('{}')) that ended up fixing it?
There was a problem hiding this comment.
Added some tests to ensure the backwards compatibility of the Date to string change!
As for the actual fix for the editor not rendering problem, what I had proposed here originally was the wrong way to go and would have only fixed the initial render while causing problems for existing notes. Turns out the cause of the issue is a race condition where the iframe onLoad event was being called before the editor called configureEditorKit. This meant that the component was being registered before there was any listener setup to handle that event, and get the rest of the flow going. It's a mystery to me though why this is only happening on iOS/Safari - perhaps the iframe is loading faster, or the event happening at a different time for some reason? At any rate, I noticed that this problem isn't happening on other editors that also make use of the editor kit and configureEditorKit, such as Markdown Visual and Token Vault. The difference is those use class components, and were calling the function either in the constructor or in componentDidMount. Here we were using useEffect, which runs slightly later only after the initial paint. Changing it to useLayoutEffect ensures the editor kit is configured earlier, which fixes the issue.
No description provided.