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

feat: added a guide for Logic Analyzer Instrument using Alert Dialog Box #944

Merged
merged 1 commit into from Jun 2, 2018
Merged

Conversation

harsh-2711
Copy link
Contributor

Fixes #943

Changes: Added documentation including schematic for Logic Analyzer using Alert Dialog Box which won't pop up when Don't show again is checked

Screenshots for the change:
logic_analyzer.zip

APK for testing:
app-debug.zip

dontShowAgain = (CheckBox) dialogView.findViewById(R.id.toggle_show_again);
final TextView tv1 = (TextView) dialogView.findViewById(R.id.custom_dialog_text);
final TextView tv2 = (TextView) dialogView.findViewById(R.id.description_text);
final ImageView iv = (ImageView) dialogView.findViewById(R.id.custom_dialog_schematic);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good Java practice is to use meaningful variable names instead of these type of naming

iv.setImageResource(iconID);
tv2.setText(desc);

final android.support.v7.app.AlertDialog dialog = builder.create();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can import these in the class imports section

@SuppressLint("ResourceType")
public void howToConnectDialog(String title, String intro, int iconID, String desc) {
try {
final android.support.v7.app.AlertDialog.Builder builder = new android.support.v7.app.AlertDialog.Builder(getContext());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can import these in the class imports section

View dialogView = inflater.inflate(R.layout.custom_dialog_box, null);

final SharedPreferences settings = getActivity().getSharedPreferences(PREFS_NAME, 0);
String skipMessage = settings.getString("skipMessage", "NOT checked");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make "NOT" to "Not"

View dialogView = inflater.inflate(R.layout.custom_dialog_box, null);

final SharedPreferences settings = getActivity().getSharedPreferences(PREFS_NAME, 0);
String skipMessage = settings.getString("skipMessage", "NOT checked");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's trivial to use a boolean in cases like this rather than a string matching.

SharedPreferences.Editor editor = settings.edit();
editor.putString("skipMessage", checkBoxResult);
editor.apply();

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole file needs a reformat.

@CloudyPadmal CloudyPadmal added this to the Milestone 1 (GSoC '18) milestone May 31, 2018
@CloudyPadmal CloudyPadmal added the Status: Review Required Requested reviews from peers and maintainers label May 31, 2018
@CloudyPadmal CloudyPadmal removed this from the Milestone 1 (GSoC '18) milestone May 31, 2018
@CloudyPadmal CloudyPadmal changed the title Added a guide for Logic Analyzer Instrument using Alert Dialog Box feat: added a guide for Logic Analyzer Instrument using Alert Dialog Box May 31, 2018
@harsh-2711
Copy link
Contributor Author

@CloudyPadmal All requested changes are done.

Copy link
Collaborator

@CloudyPadmal CloudyPadmal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're still using a string comparison to check if the check box is ticked or not. Make it a Boolean variable. Boolean variables hold only two states, true or false. If the check box is ticked, make it true. If not make it false. It's common practice.

@harsh-2711
Copy link
Contributor Author

harsh-2711 commented May 31, 2018

@CloudyPadmal Changes done. I thought strings would be OK so I kept them. But now I changed them to Boolean value

Boolean checkBoxResult = false;
if (dontShowAgain.isChecked())
checkBoxResult = true;
SharedPreferences settings = getActivity().getSharedPreferences(PREFS_NAME, 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it this SharedPreference initialized again? You can use the previously initialized variable can't you?

@harsh-2711
Copy link
Contributor Author

@Vikum94 I made the changes and the duplication was due to using final keyword.

Copy link
Collaborator

@CloudyPadmal CloudyPadmal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work! Thank you for the PR! 👍

@CloudyPadmal CloudyPadmal merged commit c0a9b1d into fossasia:development Jun 2, 2018
@CloudyPadmal CloudyPadmal removed the Status: Review Required Requested reviews from peers and maintainers label Jun 3, 2018
neel1998 pushed a commit to neel1998/pslab-android that referenced this pull request Jul 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants