Skip to content
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

Array size limited when using toga.Selection #2319

Closed
wafaa09 opened this issue Jan 3, 2024 · 7 comments · Fixed by #2326
Closed

Array size limited when using toga.Selection #2319

wafaa09 opened this issue Jan 3, 2024 · 7 comments · Fixed by #2326
Labels
bug A crash or error in behavior. macOS The issue relates to Apple macOS support.

Comments

@wafaa09
Copy link

wafaa09 commented Jan 3, 2024

Describe the bug

When creating an array, and setting said array as the items in toga.Selection, size is limited to 27 selections (specifically on iOS) before error occurs.

Steps to reproduce

  1. Create array larger than 27 items.
  2. toga.Selection(items = array)
  3. Run Program
  4. Error: 2024-01-03 16:53:10.906 python[65170:960463] *** Assertion failure in -[NSMenu insertItemWithTitle:action:keyEquivalent:atIndex:], NSMenu.m:1185

Expected behavior

Expect toga.Selection to show all items in array if larger than 27.

Screenshots

Screenshot 2024-01-03 at 5 18 46 PM

Environment

  • Operating System: iOS
  • Python version: 3.10.13
  • Software versions:
    • Briefcase: 0.3.16
    • Toga: 0.4.1

Logs

No logs available from this error.

Additional context

No response

@wafaa09 wafaa09 added the bug A crash or error in behavior. label Jan 3, 2024
@freakboy3742
Copy link
Member

To clarify - are you seeing this on macOS, or iOS, or both? The report says iOS, but the screenshot is of briefcase dev, which will be using the macOS backend.

@wafaa09
Copy link
Author

wafaa09 commented Jan 4, 2024

Oops! Mac

@freakboy3742 freakboy3742 added the macOS The issue relates to Apple macOS support. label Jan 5, 2024
@freakboy3742
Copy link
Member

Thanks - I've found the source of the problem. I'm not sure if this is bug in error handling or in the widget itself; at the very least, error handling should be improved.

The bug is entirely macOS specific; AFAICT, no other backend is affected.

The problem is with your data - it contains duplicates. There are 2 "a" entries and 2 "b" entries; the bug you're seeing is cause because the list of unique entries has less items than the list of entries. On macOS, Selection items are expected to be unique. When macOS encounters the first duplicate, it repositions the first original item to the end of the list; but at this point, the number of items in the data doesn't match the number of items in the selection, so the insertion of the second item fails because it exceeds the current size of the selection.

The immediate workaround - avoid duplicate titles in your selection data.

Longer term, I can see 2 possible fixes:

  1. Raise an error if the data contains duplicate titles
  2. Modify the implementation so it can display duplicate items.

All the other backends allow duplicate titles, which suggests that (2) is the fix required for macOS. This SO post suggests a possible approach.

However, it's worth noting that allowing duplicated entries means you end up with selections with duplicated titles that can't be differentiated. At the very least, this should probably be raised as a warning, as it indicates a possible UX issue.

For future reference, the part of this bug report that was helpful was the code sample included in the screenshot. The first part of any bug triage process is reproducing the bug; and that means having a working example. If you hadn't provided the screenshot of your code, I would have concluded this bug didn't exist, because the selection example app contains an example of a selection widget that has 72 entries. However, using as screenshot means I needed to manually type in parts of your code. Providing a fully working (but minimal) example is the most effective way to ensure someone can fix your bug.

@wafaa09
Copy link
Author

wafaa09 commented Jan 6, 2024

This is exactly the problem I was having in the three places I received the error. Was able to add an '*' or number to solve the problem for now. Thank you!

@wafaa09
Copy link
Author

wafaa09 commented Jan 6, 2024

However, it's worth noting that allowing duplicated entries means you end up with selections with duplicated titles that can't be differentiated. At the very least, this should probably be raised as a warning, as it indicates a possible UX issue.

The code I sent in this example are not the selection options I desire to use- they were merely a shorter array that I could visually see. For the actual code (the pictures attached), I am using html parse to upload current watches and warnings across the US, and based on the warning/watch, some titles would be similar. If what you said above is true, would it be possible to also add the position of the selection option as a way to get the correct output desired? Also, is this the same for all platforms in terms of not being able to differentiate between which selection is chosen when there are duplicates?

Screenshot 2024-01-05 at 7 29 34 PM Screenshot 2024-01-05 at 7 29 41 PM Screenshot 2024-01-05 at 7 29 59 PM

@freakboy3742
Copy link
Member

Side note - it's a lot easier to share text as text, rather than screenshots. You can copy code from your editor directly into the Github window, and it will be rendered as code if you include it in triple backticks:

def like_this():
    pass

As for changing the representation to be unambiguous - that's definitely possible. The literal text in the selection items doesn't need to match the actual "value" represented by the selection. There's 2 possible approaches to implementing this. Instead of specifying the items as a list of strings, use:

  1. a "list of dictionaries", and provide an accessor to define which key in the dictionary is the printable representation. When you use this, the value returned as the current selection will be the full dictionary, so you can access any other attribute of the dictionary when a value has been selected, not just the "title". An example of this is shown in the Toga docs for Selection.

  2. a "list of objects". Define a custom data storage class that has all the properties you want to store, and an implementation of __str__ that describes how the object should be rendered; then display a list of those objects. The selection pulldown will display the __str__() for each item in the list; the value returned will be the instance.

@wafaa09
Copy link
Author

wafaa09 commented Jan 7, 2024

Great info here- thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A crash or error in behavior. macOS The issue relates to Apple macOS support.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants