-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Fix checkbox widget when listIndex field undefined #7679
Fix checkbox widget when listIndex field undefined #7679
Conversation
When listIndex was pointing to an entry that didn't exist yet, so that extractTiddlerDataItem returned undefined, the checkbox widget was aborting and refusing to proceed. This is not the desired behavior: we want it to create the index if it didn't exist yet. So now we explicitly check for undefined and treat it the same as an empty list. Also fixed one code-style issue in the same file while I was in here.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
An alternative way to fix this bug would have been to change to be: fieldContents = this.wiki.extractTiddlerDataItem(this.checkboxTitle,this.checkboxListIndex,[]); @Jermolene, please let me know if you would prefer this one-line fix instead of the two-line fix I ultimately went with, and I'll change it. I decided in favor of the two-line fix because it would let me create a later PR that cleans up a bit of the code and makes one use case slightly (very slightly) more performant. (If you're wondering, it would let me stop making copies of an empty array, leading to one less small object that the browser needs to garbage-collect. A very, very small performance win, and not worth the added complexity in a PR just days before release; I'll get that PR into 5.3.2). |
@Jermolene This is a bugfix for a bug I introduced in 5.3.0, so I'd love to get it into 5.3.1 if possible. I know it's post-freeze, but since this is such an easy-to-review bugfix for a rather annoying bug (the |
Great thanks @rmunn I think that this is worth including in v5.3.1. |
When listIndex was pointing to an entry that didn't exist yet, so that extractTiddlerDataItem returned undefined, the checkbox widget was aborting and refusing to proceed. This is not the desired behavior: we want it to create the index if it didn't exist yet. So now we explicitly check for undefined and treat it the same as an empty list.
Also fixed one code-style issue in the same file while I was in here.
Fixes #7678.