fix: Changed the UI and functionality of buttons in Lux Meter #1303

harsh-2711 · 2018-07-28T23:29:29Z

Fixes #1284

Changes:

Changed the label in the BottomNavigationMenu
Changed the saved icon to record and pause icon
Added delete and export data features

Screenshot/s for the changes:
confusingBtns.zip

The updated icons screenshot: (@harsh-2711 please resize screenshots in PRs, I've done it here)

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

CloudyPadmal

For icons use xml/svg files

CloudyPadmal · 2018-07-30T11:43:05Z

app/src/main/java/org/pslab/activity/LuxMeterActivity.java

                }
-                invalidateOptionsMenu();
+                break;
+            case R.id.save_csv_data:


Change this to record_csv_data as it is changed in all other places

harsh-2711 · 2018-07-30T17:32:16Z

@CloudyPadmal All changes are done. Please review.

harsh-2711 · 2018-07-30T17:38:33Z

app/src/main/res/drawable/delete_icon.xml

+        android:viewportHeight="24.0"
+        android:tint="?attr/colorControlNormal">
+    <path
+        android:fillColor="#FFF"


The color is hard coded because referring to resources file from this file isn't supported on API <21.

Avjeet · 2018-07-30T21:15:08Z

icon is showing properly

CloudyPadmal

Icons are not sharp. Can you use vector drawables provided by Android Studio itself?

harsh-2711 · 2018-08-02T07:35:28Z

@CloudyPadmal I have used vector drawables only. You can check the extension of the file it's .xml. I have also updated the apk. Have a look.

CloudyPadmal · 2018-08-02T10:37:24Z

app/src/main/res/drawable/record_icon.xml

+        android:height="24dp"
+        android:viewportWidth="24.0"
+        android:viewportHeight="24.0"
+        android:tint="?attr/colorControlNormal">


Applying tint attributes shows a warning

Resource references will not work correctly in images generated for this vector icon for API < 21

Current min SDK is 16. Please remove this tint attribute from icons in this PR.

Sure @CloudyPadmal

Avjeet · 2018-08-02T20:47:29Z

@harsh-2711 In this PR, even If I don't click on Export data the CSV file is still getting created in my external storage, if that is the case then what is point of having export data button.

Avjeet · 2018-08-02T20:50:46Z

@harsh-2711 Can you make this PR only concerned with UI and remove the play/pause functionalities because I created them in my PR.

CloudyPadmal

Fix only the issue you have mentioned this PR is fixing. Refer to best practices.

Avjeet · 2018-08-03T23:07:09Z

@harsh-2711 what is the update on this?

harsh-2711 · 2018-08-04T10:02:07Z

@Avjeet Only changing the buttons makes no sense. I have completed working on the issue of the file being created. I will soon update the PR. Also, I will change the title of this PR.

harsh-2711 · 2018-08-04T11:29:49Z

@Avjeet @CloudyPadmal I have made the suggested changes. Please review. I have also made this suggested changes in PR #1320. Good catch @Avjeet

Avjeet · 2018-08-04T20:36:10Z

@harsh-2711 Actually the problem I mentioned can be solved by storing the data initially in some temporary database and then if the user presses export data then retrieve the data from the database and store it in CSV file which I had done in PR. Doing this will also allow us to create the Logged data activity that Mario mentioned in the previous meeting. I have made that changes in PR #1323.
That is why I asked to make this PR related to UI i.e., change the icons, remove the setting and Map option.
Moreover, in this PR we have another issue that it is difficult for the user to understand play pause if the lux meter always keeps running.

harsh-2711 · 2018-08-05T04:57:05Z

@Avjeet A user opens an instrument only when he wants to use it. Therefore, it's not logical to freeze the instrument until he clicks the play button. In this PR, the user will be able to use the instrument as soon as he opens it and will able to record the data if he wants. Also, for the database issue, I can store the data first in a database and then in a file when a user clicks Export Data if you want (Currently I am doing the same but I am storing data in a file). Else you can add this functionality in your PR #1323. And I don't think there's anything difficult for a user to understand how to use the buttons in the instrument as I have used the icons from Material Design library of Google.

…nstrument

CloudyPadmal

We'll move forward from here

Avjeet · 2018-08-07T04:01:57Z

@harsh-2711 In last meeting it has been discussed there is no need of delete and export button here in lux meter it will be shifted to logged data activity and also could you try to make a icon like this

@CloudyPadmal what do you think??

mariobehling

@harsh-2711 Please change the button in a way that users can understand that this is a recording button more easily, e.g. as proposed by @Avjeet or something similar to below.

Avjeet · 2018-08-08T12:02:17Z

@CloudyPadmal @harsh-2711 @cweitat I think the changes from this PR also got merged into the code with the PR #1333.
@harsh-2711 You shouldn't have squashed the changes in the PR #1333 until your previous PR got merged.

harsh-2711 · 2018-08-08T16:18:34Z

@Avjeet I did a rebase and reset so all the commits got squashed. But I did make a note about that in the PR body. I think @cweitat missed it.

Avjeet · 2018-08-08T16:19:41Z

@harsh-2711 Ok harsh I have made the new icon and all the changes in my PR pls review it.
So should we close this PR then ?

harsh-2711 added the In Progress Developer is working on the problem label Jul 28, 2018

CloudyPadmal requested changes Jul 29, 2018

View reviewed changes

CloudyPadmal mentioned this pull request Jul 29, 2018

lux is always running #1304

Closed

Avjeet mentioned this pull request Jul 29, 2018

feat: extended record button to start sensor data fetching and recording #1305

Merged

5 tasks

harsh-2711 self-assigned this Jul 29, 2018

harsh-2711 removed the In Progress Developer is working on the problem label Jul 30, 2018

CloudyPadmal requested changes Jul 30, 2018

View reviewed changes

harsh-2711 requested review from Avjeet, CloudyPadmal and abhinavraj23 July 30, 2018 17:19

harsh-2711 commented Jul 30, 2018

View reviewed changes

harsh-2711 added the Status: Review Required Requested reviews from peers and maintainers label Jul 30, 2018

CloudyPadmal requested changes Jul 31, 2018

View reviewed changes

CloudyPadmal requested changes Aug 2, 2018

View reviewed changes

harsh-2711 mentioned this pull request Aug 2, 2018

feat: Added toolbar with multiple features in Accelerometer instrument #1320

Closed

5 tasks

CloudyPadmal requested changes Aug 3, 2018

View reviewed changes

harsh-2711 closed this Aug 5, 2018

harsh-2711 reopened this Aug 5, 2018

harsh-2711 changed the title ~~fix: Changed the confusing UI of Lux Meter~~ fix: Changed the UI and functionality of buttons in Lux Meter Aug 5, 2018

Changed the confusing UI and implementation of buttons in Lux Meter i…

bb0f47f

…nstrument

harsh-2711 mentioned this pull request Aug 6, 2018

feat: Added bottom sheet guide for Accelerometer instrument #1333

Merged

5 tasks

CloudyPadmal approved these changes Aug 7, 2018

View reviewed changes

abhinavraj23 approved these changes Aug 7, 2018

View reviewed changes

mariobehling requested changes Aug 7, 2018

View reviewed changes

harsh-2711 closed this Aug 8, 2018

CloudyPadmal removed the Status: Review Required Requested reviews from peers and maintainers label Dec 13, 2018

fix: Changed the UI and functionality of buttons in Lux Meter #1303

fix: Changed the UI and functionality of buttons in Lux Meter #1303

Uh oh!

Conversation

harsh-2711 commented Jul 28, 2018 • edited by CloudyPadmal Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CloudyPadmal left a comment

Choose a reason for hiding this comment

Uh oh!

CloudyPadmal Jul 30, 2018

Choose a reason for hiding this comment

Uh oh!

harsh-2711 commented Jul 30, 2018

Uh oh!

harsh-2711 Jul 30, 2018

Choose a reason for hiding this comment

Uh oh!

Avjeet commented Jul 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CloudyPadmal left a comment

Choose a reason for hiding this comment

Uh oh!

harsh-2711 commented Aug 2, 2018

Uh oh!

CloudyPadmal Aug 2, 2018

Choose a reason for hiding this comment

Uh oh!

harsh-2711 Aug 4, 2018

Choose a reason for hiding this comment

Uh oh!

Avjeet commented Aug 2, 2018

Uh oh!

Avjeet commented Aug 2, 2018

Uh oh!

CloudyPadmal left a comment

Choose a reason for hiding this comment

Uh oh!

Avjeet commented Aug 3, 2018

Uh oh!

harsh-2711 commented Aug 4, 2018

Uh oh!

harsh-2711 commented Aug 4, 2018

Uh oh!

Avjeet commented Aug 4, 2018

Uh oh!

harsh-2711 commented Aug 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CloudyPadmal left a comment

Choose a reason for hiding this comment

Uh oh!

Avjeet commented Aug 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mariobehling left a comment

Choose a reason for hiding this comment

Uh oh!

Avjeet commented Aug 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

harsh-2711 commented Aug 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Avjeet commented Aug 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

harsh-2711 commented Jul 28, 2018 •

edited by CloudyPadmal

Loading

Avjeet commented Jul 30, 2018 •

edited

Loading

harsh-2711 commented Aug 5, 2018 •

edited

Loading

Avjeet commented Aug 7, 2018 •

edited

Loading

Avjeet commented Aug 8, 2018 •

edited

Loading

harsh-2711 commented Aug 8, 2018 •

edited

Loading

Avjeet commented Aug 8, 2018 •

edited

Loading