The top (first) fix is to prevent an NPE when the selection is set to a value that isn't in the list. Some may argue that an Err should be thrown as you shouldn't attempt to set a value that's not in the list, but then again, you're not always in complete control of the values your editing, so a lenient default selection would be much nicer.
Either way, the current NPE is confusing and definitely wrong.
The second (bottom) fix is one I'm sure I've mentioned in the past but doesn't seem to have been adopted yet; and simply corrects a syntax typo.
diff -r 07854a5c8ab9 src/domkit/fan/ListButton.fan
--- a/src/domkit/fan/ListButton.fan Mon Jan 28 11:17:06 2019 -0500
+++ b/src/domkit/fan/ListButton.fan Thu Jan 31 19:57:51 2019 +0000
@@ -58,7 +58,8 @@
if (isCombo) return
this.removeAll
if (items.size == 0) this.add(Elem { it.text = "\u200b" })
- else this.add(makeElem(sel.item))
+ // one can't be certain a valid selection exists - so prevent a meanlingless NPE
+ else if (sel.item != null) this.add(makeElem(sel.item))
}
** Fire select event.
@@ -119,7 +120,7 @@
new make(ListButton button) { this.button = button }
override Int max() { button.items.size }
override Obj toItem(Int index) { button.items[index] }
- override Int? toIndex(Obj item) { button.items.findIndex(item) }
+ override Int? toIndex(Obj item) { button.items.findIndex { it == item } }
override Void onUpdate(Int[] oldIndexes, Int[] newIndexes) { button.update }
private ListButton button
andyFri 1 Feb 2019
Ticket promoted to #2730 and assigned to andy
Thanks Steve
I pushed a fix for the findIndex
Let me sleep on the first one; might handle that case a bit different
andyMon 4 Feb 2019
Ticket resolved in 1.0.72
I tweaked this a bit to guarantee content always get added: e88bf63
SlimerDude Thu 31 Jan 2019
Two fixes for ListButton:
The top (first) fix is to prevent an NPE when the selection is set to a value that isn't in the list. Some may argue that an Err should be thrown as you shouldn't attempt to set a value that's not in the list, but then again, you're not always in complete control of the values your editing, so a lenient default selection would be much nicer.
Either way, the current NPE is confusing and definitely wrong.
The second (bottom) fix is one I'm sure I've mentioned in the past but doesn't seem to have been adopted yet; and simply corrects a syntax typo.
andy Fri 1 Feb 2019
Ticket promoted to #2730 and assigned to andy
Thanks Steve
I pushed a fix for the
findIndex
Let me sleep on the first one; might handle that case a bit different
andy Mon 4 Feb 2019
Ticket resolved in 1.0.72
I tweaked this a bit to guarantee content always get added: e88bf63
Thanks for the patch Steve.