Activity data dividers #4294

AlexNi245 · 2019-08-04T22:11:18Z

Resolves #2216

This PR belongs to the ActivitiesAction. The date string is now sticked to the top above the RecylerView, ts has the value of first visible item in the view.

The data string is now fully generated in the ActivitiesActivity and the ActivityListAdapter now only deals with Activities instead with Objects.
In my opinion it makes sense to change the signature of
ActivitiesContract.showActivities(List<Object> activities, OwnCloudClient client, String nextPageUrl)
to ActivitiesContract.showActivities(List<Activities> activities, OwnCloudClient client, String nextPageUrl) because no type casting is necessary anymore.

Last but not least i added some white space around the headline.

I would appreciate any improvement proposal

AndyScherzinger · 2019-08-05T13:56:59Z

Hi @AlexNi245,

first of all thank you for this PR, I tested it and it does what the description of the PR is saying but not what the issue #2216 requires, namely "sticky headers" which is a defined UI behavior meaning the actuall headers (kind of like section header) that have been in placce before need to be made "sticky" which means that while scrolling the actual header needs to stick at the top and will be pushed out be the next header when scrolling the list. In contrast while your PR displays the actual header at the top the list now lost all its headers! :(

An examplpe would be https://github.com/bgogetap/StickyHeaders (see readme's animaed gif while this library unfortunately only supports Android v15+ (while Nextcloud supports 14+ so that won't work)

AlexNi245 · 2019-08-05T15:52:40Z

Thank you for your feedback @AndyScherzinger. Obviously i got the issue wrong. I will change the behavior of the header like the way it behaves it the example that you mentioned.

tobiasKaminsky · 2019-08-07T06:04:46Z

@AlexNi245 thanks for your great PR so far.
If you need any help or have questions, do not hesitate to ask directly here.

codecov · 2019-08-07T16:31:24Z

Codecov Report

Merging #4294 into master will decrease coverage by 0.53%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master    #4294      +/-   ##
============================================
- Coverage     16.61%   16.07%   -0.54%     
  Complexity        1        1              
============================================
  Files           351      341      -10     
  Lines         31397    31145     -252     
  Branches       4446     4422      -24     
============================================
- Hits           5216     5008     -208     
+ Misses        25306    25272      -34     
+ Partials        875      865      -10

Impacted Files	Coverage Δ	Complexity Δ
...roid/ui/fragment/FileDetailActivitiesFragment.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...loud/android/ui/activities/ActivitiesActivity.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...ncloud/android/ui/adapter/ActivityListAdapter.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...roid/ui/adapter/ActivityAndVersionListAdapter.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...d/android/ui/interfaces/ActivityListInterface.java	`0% <0%> (ø)`	`0 <0> (?)`
...m/owncloud/android/ui/adapter/AccountListItem.java	`0% <0%> (-16.67%)`	`0% <0%> (ø)`
...wncloud/android/ui/adapter/AccountListAdapter.java	`0% <0%> (-10.38%)`	`0% <0%> (ø)`
...ud/android/ui/activity/ManageAccountsActivity.java	`0% <0%> (-0.55%)`	`0% <0%> (ø)`
...cloud/android/ui/activity/FileDisplayActivity.java	`20.51% <0%> (-0.17%)`	`0% <0%> (ø)`
src/main/java/com/owncloud/android/MainApp.java	`54.96% <0%> (-0.15%)`	`0% <0%> (ø)`
... and 19 more

codecov · 2019-08-07T16:31:24Z

Codecov Report

Merging #4294 into master will decrease coverage by 0.09%.
The diff coverage is 0%.

@@             Coverage Diff             @@
##             master    #4294     +/-   ##
===========================================
- Coverage     16.85%   16.76%   -0.1%     
  Complexity        1        1             
===========================================
  Files           357      355      -2     
  Lines         31727    31577    -150     
  Branches       4482     4483      +1     
===========================================
- Hits           5349     5294     -55     
+ Misses        25483    25391     -92     
+ Partials        895      892      -3

Impacted Files	Coverage Δ	Complexity Δ
...roid/ui/activities/ActivityListItemDecoration.java	`0% <0%> (ø)`	`0 <0> (?)`
...loud/android/ui/activities/ActivitiesActivity.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...ncloud/android/ui/adapter/ActivityListAdapter.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...n/java/com/nextcloud/client/device/DeviceModule.kt	`83.33% <0%> (-2.39%)`	`0% <0%> (ø)`
...ncloud/android/ui/fragment/OCFileListFragment.java	`24.73% <0%> (-1.39%)`	`0% <0%> (ø)`
src/main/java/com/owncloud/android/MainApp.java	`54.54% <0%> (-1.36%)`	`0% <0%> (ø)`
...in/java/com/owncloud/android/utils/ThemeUtils.java	`51.89% <0%> (-1.36%)`	`0% <0%> (ø)`
...ava/com/owncloud/android/ui/EmptyRecyclerView.java	`61.76% <0%> (-1.1%)`	`0% <0%> (ø)`
...ain/java/com/nextcloud/client/logger/LoggerImpl.kt	`86.36% <0%> (-0.6%)`	`0% <0%> (ø)`
...loud/android/datamodel/ThumbnailsCacheManager.java	`35.66% <0%> (-0.56%)`	`0% <0%> (ø)`
... and 22 more

AndyScherzinger · 2019-08-07T18:03:53Z

Unfortunately the build is broken due to a compile error

/src/main/java/com/owncloud/android/ui/activities/ActivityHeaderPositioner.java:33: error: ';' expected

AlexNi245 · 2019-08-08T14:34:44Z

Yes its still work in progress. I mention you when iam done. Is there a label or something to indicate that there is work in progress?

tobiasKaminsky · 2019-08-08T14:46:06Z

Is there a label or something to indicate that there is work in progress?

Label "1. developing".
Once you are ready, you can switch it to "2. to review".

AndyScherzinger · 2019-08-08T19:21:46Z

@AlexNi245 can you change the alignment of the header back to their original state as in vertically aligned with the toolbar's title and the activity item text? That be great ❤️

nextcloud-android-bot · 2019-08-08T21:54:29Z

IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/10326

AlexNi245 · 2019-08-08T21:55:04Z

@AndyScherzinger i reverted the branch to the previous version.I also merged the current master into it and create 3 classes related to the stickyHeaders feature. At the moment they have no affects and the app behaves like the version from the master branch.

do you think it make sense to left the pr open for discussion, or should i open a new one as soon as the feature works correctly ?

AndyScherzinger · 2019-08-09T21:53:49Z

Any reason you closed the PR?

AndyScherzinger · 2019-08-10T07:27:16Z

@AlexNi245 Sorry I didn't see you question. Both would be fine actually. I kind of depends rather on when you want your work to be visibility, to have discussions and also get feedback from the CI job (code coverage, code analysis etc).
So a new want would be fine I think, I also think opening an PR early on makes sense to other see that you are actively working on the matter so they won't start in parallel (very rare case).