Skip to content

Conversation

@ho-dor
Copy link

@ho-dor ho-dor commented May 31, 2019

Fixes #1756

Changes: Settings menu with the option to change units.

Screenshot/s for the changes:
thermo_gif

Checklist: [Please tick following check boxes with [x] if the respective task is completed]

  • I have used resources from strings.xml, dimens.xml and colors.xml without hard-coding them
  • No modifications done at the end of resource files strings.xml, dimens.xml or colors.xml
  • I have reformatted code in every file included in this PR [CTRL+ALT+L]
  • My code does not contain any extra lines or extra spaces
  • I have requested reviews from other members

APK for testing:
app-debug.zip

@ho-dor
Copy link
Author

ho-dor commented May 31, 2019

@CloudyPadmal @neel1998 please review.

label_statAvg.setText(R.string.avg_thermo_fahrenheit);
label_statMax.setText(R.string.max_thermo_fahrenheit);
label_statMin.setText(R.string.min_thermo_fahrenheit);
thermometer.setUnit("F");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Put the degree sign to this as well similar to C

public static final String KEY_INCLUDE_LOCATION = "include_location_sensor_data";
public static final String KEY_UPDATE_PERIOD = "setting_thermo_update_period";
public static final String KEY_THERMO_SENSOR_TYPE = "setting_thermo_sensor_type";
public static final String KEY_THERMO_SENSOR_GAIN = "setting_thermo_sensor_gain";
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't have a "Gain" for thermo sensor. Can remove this setting.

</string-array>

<string-array name="thermo_unit_list">
<item>F</item>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Degree sign

@CloudyPadmal CloudyPadmal added the Status: Review Required Requested reviews from peers and maintainers label Jun 1, 2019
@ho-dor
Copy link
Author

ho-dor commented Jun 1, 2019

@CloudyPadmal check

@neel1998
Copy link
Member

neel1998 commented Jun 1, 2019

@ho-dor when the user opens the configuration for the first time. No unit is shown in select unit preference. Please cet C as default. Also add space in the menu title "Thermometer Configuration"

@ho-dor
Copy link
Author

ho-dor commented Jun 1, 2019

@ho-dor when the user opens the configuration for the first time. No unit is shown in select unit preference. Please cet C as default. Also add space in the menu title "Thermometer Confguration"

The menu title is actually consistent with all the other instruments.

@ho-dor
Copy link
Author

ho-dor commented Jun 1, 2019

I have the updated the default unit value . Please check the updated apk.

@CloudyPadmal CloudyPadmal requested a review from neel1998 June 1, 2019 08:23
@CloudyPadmal CloudyPadmal merged commit ed9f70c into fossasia:development Jun 1, 2019
@CloudyPadmal CloudyPadmal removed the Status: Review Required Requested reviews from peers and maintainers label Jun 1, 2019
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.

Adding the settings menu for thermometer

3 participants