-
Notifications
You must be signed in to change notification settings - Fork 73
UI: disabled the positive button for null validation and made default hotspot name as device name #348
base: master
Are you sure you want to change the base?
Conversation
@@ -0,0 +1,73 @@ | |||
package org.odk.share.views.ui.settings; |
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.
We should use some names that can indicate the class usage, not just CustomEditTextPreference
@Chromicle .
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.
I gave that name because maybe it will be useful for further implementations of any features
* @author by Chromicle ([email protected]) | ||
* @since 1/30/2020 | ||
*/ | ||
public class CustomEditTextPreference extends EditTextPreference implements DialogInterface.OnClickListener { |
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.
I'm not sure do we really need a custom preference for this tiny check? @Chromicle
Can we use some method like,
private void checkNullEditTextPreference(EditTextPreference edttxtpref) {
edttxtpref.setOnPreferenceChangeListener(new OnPreferenceChangeListener() {
@Override
public boolean onPreferenceChange(
android.preference.Preference preference, Object newValue) {
if (newValue.toString().trim().equals("")) {
// Get Dialog and disable button here
return false;
}
return true;
}
});
}
and receiving a EditTextPreference? You know, building a custom class is always a heavy work and not good for code reading and code refactoring. I didn't test for this method but you can have a try.
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.
As far as I know, setOnPreferenceChangeListener
will be triggered when we press the positive button of the dialog so there is no use if we disable the button then so, here we want to check the edit text for every time so we have to implement the editText watcher
correct me if I am wrong
Now I updated the code without implementing the customEditTextPreference
can please have a look now and thanks for your suggestions 😄
7c80232
to
92e6549
Compare
@@ -184,4 +181,6 @@ | |||
<string name="finalized_on_date_at_time">\'Finalized on\' EEE, MMM dd, yyyy \'at\' HH:mm</string> <!-- http://docs.oracle.com/javase/6/docs/api/java/text/SimpleDateFormat.html --> | |||
<string name="sent_on_date_at_time">\'Sent on\' EEE, MMM dd, yyyy \'at\' HH:mm</string> <!-- http://docs.oracle.com/javase/6/docs/api/java/text/SimpleDateFormat.html --> | |||
<string name="sending_failed_on_date_at_time">\'Sending failed on\' EEE, MMM dd, yyyy \'at\' HH:mm</string> <!-- http://docs.oracle.com/javase/6/docs/api/java/text/SimpleDateFormat.html --> | |||
|
|||
<string name="one_space">\u0020</string> |
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.
There is no need to create a string resources for space.
hotspotNamePreference.setSummary(prefs.getString(PreferenceKeys.KEY_HOTSPOT_NAME, | ||
getString(R.string.default_hotspot_ssid))); | ||
|
||
String defaultHotspotName = getString(R.string.app_name) + getString(R.string.one_space) + Build.MODEL; |
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.
If this should just be a device odk-skunkworks-crow or the device name. Not a combination of both because if try to keep both the combinations, then it may get bigger for few devices then it may end up truncating or creating a double line for displaying just the name of the bluetooth/hotspot device.
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.
can we go with device name
Closes #347 #136
What has been done to verify that this works as intended?
Why is this the best possible solution? Were any other approaches considered?
I implemented the editTextWatcher to check for every text change
In the method of
onEditTextChanged
checked if the length of the editText is zero then I am disabling the positive button of the alert dialog, and I wrote the tests according to thatI made the one more change that I changed the
CheckBoxPreference
toSwitchPreference
as according to material design switches are used to toggle the state of a single setting on or off and checkboxes are used for select one or more items from a set like selecting theforms
For default name of hotspot as @shobhitagarwal1612 suggested I made that as
skunkworks-crow deviceName
in my case it isskunkworks-crow SM-J810G
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
By adding this feature user can directly know that he did not have to give the
null
value in edit text or else he has to select the preference once again then he has to edit and saveBy making default hotspot name as the device name so it will be like unique
GIF
Before submitting this PR, please make sure you have:
./gradlew checkCode
and confirmed all checks still pass OR confirm CircleCI build passes