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

Include switch to allow or disallow light filtering from other apps like Twilight or similars #2429

Merged
merged 9 commits into from
Feb 22, 2019

Conversation

@davigonz davigonz changed the title Include switch to allow or disallow ligth filtering from other apps Include switch to allow or disallow light filtering from other apps like Twilight or similars Jan 23, 2019
@davigonz davigonz self-assigned this Jan 29, 2019
@davigonz
Copy link
Contributor Author

davigonz commented Jan 29, 2019

@hosy @SD1998 @hannesa2 you can review this PR as well, do not panic about the amount of changes, many of them are from automatic code reformat

@davigonz davigonz requested a review from hosy January 29, 2019 16:39
@hannesa2
Copy link
Contributor

I know this issue about amount of changes on non-well formated code.
I at least split format changes and logic to different commits to make it for reviewers a more easy task.
That's why I split PRs into codeformat, lint and logic.
Advantage: codeformat is a no-brainer but causes often the biggest changers and could be done much faster

I warmly recommend to split, so here is my suggestion:

  • squash all
  • open commit git reset --soft HEAD^
  • start again with git add by first git reset .
  • add partial changes to individual commits git add . -p

then it's much easier and I will do

@hannesa2
Copy link
Contributor

hannesa2 commented Jan 29, 2019

Ok, I tried, but I gave up. Owncloud needs first a well formated code !
These PR's are the worst, where logic changes are camouflaged by codestyle changed >100 files

Doing so, we have to

  1. merge Android Studio 3.3 android-library#228 to have an every developer machine same Android Studio
  2. merge Codestyle android-library#224 to have same codstyle (a lot of changes but this is a I've-nothing-to-think-about)
  3. merge Android Studio 3.3 #2424 to have an every developer machine same Android Studio
  4. I will create codestyle PR in https://github.com/owncloud/android (this is a I've-nothing-to-think-about too)
  5. rebase this PR on new master
  6. squash all
  7. open commit git reset --soft HEAD^
  8. start again with git add by first git reset .
  9. add partial changes to individual commits git add . -p

@davigonz davigonz force-pushed the feature/light_filter_switch branch 2 times, most recently from 2fc3195 to 0c9d490 Compare February 1, 2019 10:06
@davigonz
Copy link
Contributor Author

davigonz commented Feb 1, 2019

@hannesa2 just got rid of most of the format changes, I could not delete some of them because appeared along with logic changes in the hunks while using git add . -p

I will create codestyle PR in https://github.com/owncloud/android (this is a I've-nothing-to-think-about too)

Let's face the format changes there then

@davigonz
Copy link
Contributor Author

davigonz commented Feb 6, 2019

4. I will create codestyle PR in https://github.com/owncloud/android (this is a I've-nothing-to-think-about too)

@hannesa2 codestyle in library and Android Studio 3.3 in both app and library are already merged, can you please create a codestyle PR in https://github.com/owncloud/android as you said before?

@hosy
Copy link
Collaborator

hosy commented Feb 12, 2019

@davigonz please can you resolve the conflicts, before we start a review?

@davigonz davigonz force-pushed the feature/light_filter_switch branch from 0c9d490 to e6755e6 Compare February 12, 2019 15:52
@davigonz
Copy link
Contributor Author

davigonz commented Feb 12, 2019

please can you resolve the conflicts, before we start a review?

@hosy done

@davigonz davigonz requested a review from hannesa2 February 12, 2019 15:52
@davigonz davigonz force-pushed the feature/light_filter_switch branch from e6755e6 to af8f416 Compare February 13, 2019 16:11
@davigonz
Copy link
Contributor Author

More conflicts, fixing them...

@davigonz davigonz force-pushed the feature/light_filter_switch branch 3 times, most recently from af89d8c to b11791c Compare February 18, 2019 07:43
@davigonz davigonz requested a review from abelgardep February 18, 2019 07:59
@davigonz
Copy link
Contributor Author

@hosy @abelgardep @hannesa2 can you please have a look at this PR, approving it or requesting some changes if needed? Thanks

@davigonz davigonz mentioned this pull request Feb 20, 2019
@davigonz davigonz requested a review from jesmrec February 20, 2019 15:24
@davigonz davigonz force-pushed the feature/light_filter_switch branch from b11791c to 3a6a3c1 Compare February 21, 2019 07:50
@jesmrec
Copy link
Collaborator

jesmrec commented Feb 21, 2019

Let's QA this by checking the option enabled allows tapping everywhere with filter.

@jesmrec
Copy link
Collaborator

jesmrec commented Feb 21, 2019

(1) [FIXED]

  1. Twilight enabled and option "Touches with other visible windows" enabled
  2. Settings -> enable camera uploads -> Camera folder

Current: Taps not allowed in the picker
Expected: Taps allowed

Nexus 6P v7

@jesmrec
Copy link
Collaborator

jesmrec commented Feb 21, 2019

(2) [FIXED]

Passcode lock crashes when it is enabled, regardless any other option

Stacktrace:


11:04:50.743 25790-25790/com.owncloud.android.debug E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.owncloud.android.debug, PID: 25790
    java.lang.RuntimeException: Unable to start activity ComponentInfo{com.owncloud.android.debug/com.owncloud.android.ui.activity.PassCodeActivity}: java.lang.NullPointerException: Attempt to invoke virtual method 'void android.view.View.setFilterTouchesWhenObscured(boolean)' on a null object reference

@jesmrec
Copy link
Collaborator

jesmrec commented Feb 21, 2019

(3) [FIXED]

No necessary the filter enabled

  1. Select a file in multiselection mode
  2. Select "Details"

Current: crash
Expected: details view

@jesmrec
Copy link
Collaborator

jesmrec commented Feb 21, 2019

(4) [FIXED]

No necessary the filter enabled

  1. Tap on a txt file in order to download it

Current: crash
Expected: file opened

Stack trace:

E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.owncloud.android.debug, PID: 1527
    java.lang.NullPointerException: Attempt to invoke virtual method 'android.view.View android.app.Activity.findViewById(int)' on a null object reference
        at com.owncloud.android.ui.fragment.FileDetailFragment.<init>(FileDetailFragment.java:110)
        at com.owncloud.android.ui.fragment.FileDetailFragment.newInstance(FileDetailFragment.java:90)
        at com.owncloud.android.ui.activity.FileDisplayActivity.startSyncThenOpen(FileDisplayActivity.java:1854)
        at com.owncloud.android.ui.fragment.OCFileListFragment.onItemClick(OCFileListFragment.java:821)

@jesmrec
Copy link
Collaborator

jesmrec commented Feb 21, 2019

(5) [FIXED]

open FAB -> Upload -> Files

Current: App crashes
Expected: file picker

Stacktrace:

E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.owncloud.android.debug, PID: 6855
    java.lang.RuntimeException: Unable to start activity ComponentInfo{com.owncloud.android.debug/com.owncloud.android.ui.activity.UploadFilesActivity}: java.lang.NullPointerException: Attempt to invoke virtual method 'void android.view.View.setFilterTouchesWhenObscured(boolean)' on a null object reference

@jesmrec
Copy link
Collaborator

jesmrec commented Feb 21, 2019

(6)

Uploads view is obscured by the light filter. Not posible to click in the cells, remove items from the list etc...

@davigonz davigonz force-pushed the feature/light_filter_switch branch from 6215ad2 to 7fade16 Compare February 22, 2019 08:04
@jesmrec
Copy link
Collaborator

jesmrec commented Feb 22, 2019

All stuff fixed. Approved on my side.

@davigonz davigonz merged commit bfdf35f into master Feb 22, 2019
@delete-merged-branch delete-merged-branch bot deleted the feature/light_filter_switch branch February 22, 2019 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants