[TIMOB-27249] Android: OptionDialog without radio buttons should not auto-set "selectedIndex" property after clicking option
GitHub Issue | n/a |
---|---|
Type | Improvement |
Priority | Medium |
Status | Closed |
Resolution | Fixed |
Resolution Date | 2019-08-21T13:58:10.000+0000 |
Affected Version/s | n/a |
Fix Version/s | Release 8.1.1 |
Components | Android |
Labels | OptionDialog, android, dialog, index |
Reporter | Joshua Quick |
Assignee | Yordan Banev |
Created | 2019-07-17T03:29:58.000+0000 |
Updated | 2019-08-21T13:58:11.000+0000 |
Description
*Summary:*
As of Titanium 8.1.0, Android now supports displaying an
OptionDialog
without radio buttons if its "selectedIndex" property is not set (or set to -1
). See: [TIMOB-26793]
But one usability issue with the current implementation is that tapping on an OptionDialog
button causes the "selectedIndex" to be set to the index of the tapped button. The issue with this is if the OptionDialog
is re-shown, it'll show radio buttons instead of plain buttons which is not desired. This happens because the "selectedIndex" was set.
*Proposed Solution:*
If the OptionDialog
is not showing radio buttons, then it should not automatically set the "selectedIndex" property when tapping a button. The "click" event already provides the index of the button that was tapped on and this is enough. Besides, iOS does not support the "selectedIndex" property anyways, making this behavior portable.
However, if the developer does set "selectedIndex" via code to show radio buttons, then the OptionDialog
should maintain the old behavior and update the "selectedIndex" property with the clicked button index.
*Steps to reproduce:*
Build and run the below code on Android.
Tap on the "Show Dialog" button.
Tap on the dialog's "Option 1" button.
Tap on the "Show Dialog" button again to re-show it.
Notice the dialog shows radio buttons in front of every option button. _(This is the usability issue.)_
var dialog = Ti.UI.createOptionDialog({
title: "Option Dialog",
options: ["Option 1", "Option 2", "Cancel"],
cancel: 2,
});
dialog.addEventListener("click", function(e) {
Ti.API.info("@@@ Dialog 'click' index: " + e.index + ", button: " + e.button + ", cancel: " + e.cancel);
});
dialog.addEventListener("cancel", function(e) {
Ti.API.info("@@@ Dialog 'cancel'");
});
var window = Ti.UI.createWindow();
var button = Ti.UI.createButton({ title: "Show Dialog" });
button.addEventListener("click", function(e) {
dialog.show();
});
window.add(button);
window.open();
*Work-around:*
Set "selectedIndex" to -1
before re-showing the dialog.
dialog.selectedIndex = -1;
dialog.show();
FYI: [~michael] Keeping you in the loop since you've implemented this feature. Doing this via "selectedIndex" set to
null
/-1
was my idea and it was my mistake for not noticing the usability issue to begin with.[~jquick] Thanks for the info. Are you going to have a look at a fix or shall I give it a go?
We're thinking about making this change for 8.1.0.GA. If you're willing to make this change this week, then please feel free to do so. Otherwise I'll get someone on our end to do it. Thanks.
PR (master): https://github.com/appcelerator/titanium_mobile/pull/11059
FR Passed. waiting for 8.1.0 GA to merge this PR. Also back port PR is needed .
Merged to master; cherry-picked to 8_3_X and 8_1_X
*Closing ticket* fix verified in SDK version
8.2.0.v20190820104021
,8.1.1.v20190820143437
and8.3.0.v20190820103430
. Test and other information can be found at: PR (master): https://github.com/appcelerator/titanium_mobile/pull/11059