-
-
Notifications
You must be signed in to change notification settings - Fork 43
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: control getShorthand returns array-valued property #170
Conversation
Hi @ph-alsvik - changes look good to me! |
1 similar comment
Hi @ph-alsvik - changes look good to me! |
ps: happy to blueprint a test case if needed. |
1 similar comment
ps: happy to blueprint a test case if needed. |
Hi @vobu ! Sure, I'll make a test. Should have to add in the MultiComboBox control to the ui5-js app, right? |
yep, exactly! just put it any place, layout is not the primary concern here 🤣 |
Hope this test covers it! Just let me know if I should alter it :) |
will do! |
|
Hi @vobu! Hope I did everything right (fairly new to Git). |
you're doing great then!
Please look through them and make sure, none of the recent changes in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
excellent 🤝
Very simple fix to when get$Shorthand-method returns a control prop that is an array of values (for example when calling getSelectedKeys on a MultiComboBox-control which returns an array of strings).
Please correct me on this, but AFAIK there is no way of knowing from the get$Shorthand what kind of return value is expected, other than calling the method and inspecting the returned value. This creates an edge case when the array is empty, because we don’t know if its an array of aggregates or js primitives. Therefore I’ve created a separate case “empty” for when the the returned value is an empty array, and warn the user that there is no data found in prop or aggregation.
Perhaps there is a more elegant way to address this issue?