#2730 BugFixes for domkit::ListButton

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.

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
     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

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.

Login or Signup to reply.