-
Notifications
You must be signed in to change notification settings - Fork 73
Feat : added flash light while scanning QRcode #240
base: master
Are you sure you want to change the base?
Conversation
skunkworks_crow/src/main/java/org/odk/share/activities/ScannerActivity.java
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,5 @@ | |||
<vector android:height="24dp" android:tint="#ffffff" |
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 you've used any external open source library or some other source in creating the asset, then also mention in the open_source_licenses file, and if you referred any code from some external source you should also mention that as well.
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.
No I didnot use any open source library I find the solution in stack overflow for implementing flash light ,other layout things I implemented
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 thought you used an asset from some other source, and even if you're referring to any other source its always good to add the source link in the code, it helps other developers in understanding things better.
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.
yeah, I updated in PR template and I changed the object name also
my builds are failing due to changes of #230
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.
so I made some changes to #230 can you please review now
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 some redundancy to this vector. It is filled with black color and then tinted with white. Shouldn't it be filled with white and then you wouldn't require a tint at all
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.
@Chromicle Can you add the link of stackoverflow in the code itself maybe in the activity wherever you have used the snippet, this way it can be referred easily.
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.
Yeah, I will Do it now
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.
Yeah, Done can you have a look once now
This PR contains a lot of commits, @Chromicle try squashing your commits. |
c866f51
to
c47e780
Compare
skunkworks_crow/src/main/java/org/odk/share/activities/ScannerActivity.java
Outdated
Show resolved
Hide resolved
skunkworks_crow/src/main/java/org/odk/share/activities/ScannerActivity.java
Outdated
Show resolved
Hide resolved
skunkworks_crow/src/main/java/org/odk/share/activities/ScannerActivity.java
Outdated
Show resolved
Hide resolved
skunkworks_crow/src/main/java/org/odk/share/activities/ScannerActivity.java
Outdated
Show resolved
Hide resolved
skunkworks_crow/src/main/java/org/odk/share/activities/ScannerActivity.java
Outdated
Show resolved
Hide resolved
skunkworks_crow/src/main/java/org/odk/share/activities/ScannerActivity.java
Outdated
Show resolved
Hide resolved
skunkworks_crow/src/main/java/org/odk/share/activities/ScannerActivity.java
Outdated
Show resolved
Hide resolved
skunkworks_crow/src/main/java/org/odk/share/activities/ScannerActivity.java
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,5 @@ | |||
<vector android:height="24dp" android:tint="#ffffff" |
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 some redundancy to this vector. It is filled with black color and then tinted with white. Shouldn't it be filled with white and then you wouldn't require a tint at all
skunkworks_crow/src/main/res/drawable/ic_flash_on_black_24dp.xml
Outdated
Show resolved
Hide resolved
@shobhitagarwal1612 @lakshyagupta21 can you review now |
2248569
to
a7a6e0d
Compare
skunkworks_crow/src/main/java/org/odk/share/activities/ScannerActivity.java
Outdated
Show resolved
Hide resolved
skunkworks_crow/src/main/java/org/odk/share/preferences/SettingsPreference.java
Outdated
Show resolved
Hide resolved
|
||
//Initialize barcode scanner view | ||
|
||
decoratedScannerView = findViewById(R.id.zxing_barcode_scanner); |
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.
That might happen if you are not binding views using Butterknife.bind(this)
before using any injected variable.
In this case, you might be trying to setTorchListener
. So, the solution is to do the binding first
|
||
@OnClick(R.id.switch_flashlight) | ||
public void toggleButton() { | ||
if (isFlashlightSupported) { |
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.
This if
condition will always be true
. In onCreate()
, you are setting the visibility to GONE
if the feature isn't available. Hence, such a case where the button is clicked when the flashlight isn't available is not possible
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.
This
if
condition will always betrue
. InonCreate()
, you are setting the visibility toGONE
if the feature isn't available. Hence, such a case where the button is clicked when the flashlight isn't available is not possible
Yeah, I didn't notice 🤪 , I updated it now
@lakshyagupta21 can you please review this PR also |
@Chromicle It's a great feature, please resolve the conflicts. And you can squash some commits that have no more senses. |
Added Flash light feature autofills ignored removed empty line Added the flash feature added margin and made some changes Fix crash on start Also removes duplicate write permissions requests Stop app crash on entering received forms (getodk#237) Improve UX for setting password (getodk#230) Removed unused changes Removed unused changes Introduced flashlight made maifest changes Checkstyle changed Checkstyle changed Checkstyle changed Checkstyle changed Checkstyle changed Checkstyle changed changed to QRcode Fix crash on start Also removes duplicate write permissions requests Stop app crash on entering received forms (getodk#237) Improve UX for setting password (getodk#230) Made object name to camel case Fix crash on start Also removes duplicate write permissions requests Stop app crash on entering received forms (getodk#237) Improve UX for setting password (getodk#230) Changes for getodk#230 Changed QR code camel case Changed QR code camel case
fdd9884
to
82c7e2c
Compare
Yeah I did, I wrote tests for scannerActivity also can you please check that @lakshyagupta21 @huangyz0918 |
Hello!! May I work on this issue ? |
Hello @soumyajit1729, I made the changes as suggested and it is not yet reviewed can you find some other open issues 😄 |
Closes #239
What has been done to verify that this works as intended?
Why is this the best possible solution? Were any other approaches considered?
I made a new class
ScannerActivity
in that implemented theDecoratedBarcodeView.TorchListener
so it can turn on or off by user inputmade 2 drawable files for flashlight button(on state and off state) instead of direct buttons
and changed the
setCaptureActivity
toScannerActivity
it is working fine as expectedI find the solution in one of the stack overflow posts is here
How does this change affect users?
By flashlight, if the user wants the flash then he will turn on or else he can turn off so the scanning of QR code can be done easily
GIF
Before submitting this PR, please make sure you have:
./gradlew checkCode
and confirmed all checks still pass OR confirm CircleCI build passes