Skip to content
This repository has been archived by the owner on Apr 9, 2021. It is now read-only.

UI: disabled the positive button for null validation and made default hotspot name as device name #348

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ajay-prabhakar
Copy link
Contributor

@ajay-prabhakar ajay-prabhakar commented Jan 30, 2020

Closes #347 #136

What has been done to verify that this works as intended?

  • Checked whether the positive button is disabling when the length of the text is null
  • Checked any new warnings are producing or failing any tests, working perfectly as expected
  • Checked by changing the orientation
  • Checked the default name for the hotspot is device name or not

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 that
I made the one more change that I changed the CheckBoxPreference to SwitchPreference 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 the forms

For default name of hotspot as @shobhitagarwal1612 suggested I made that as skunkworks-crow deviceName in my case it is skunkworks-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 save
By making default hotspot name as the device name so it will be like unique

GIF

ezgif com-video-to-gif (2)

Before submitting this PR, please make sure you have:

  • run ./gradlew checkCode and confirmed all checks still pass OR confirm CircleCI build passes
  • verified that any code or assets from external sources are properly credited in comments and/or in the about file.

@ajay-prabhakar ajay-prabhakar changed the title Def name UI: disabled the positive button for null validation and made default hotspot name as device name Jan 30, 2020
@@ -0,0 +1,73 @@
package org.odk.share.views.ui.settings;
Copy link
Contributor

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 .

Copy link
Contributor Author

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 (ajayprabhakar369@gmail.com)
* @since 1/30/2020
*/
public class CustomEditTextPreference extends EditTextPreference implements DialogInterface.OnClickListener {
Copy link
Contributor

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.

Copy link
Contributor Author

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 😄

@@ -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>
Copy link
Contributor

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;
Copy link
Contributor

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.

Copy link
Contributor Author

@ajay-prabhakar ajay-prabhakar Feb 18, 2020

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disable button instead of showing toast for null validation
3 participants