davivel commented 9 years ago

Google states that the new elements for lists and grid view included in the Android Support Library help to improve the performance of lists and grids with tons of elements. I couldn't check it, but it's obvious they enforce the recycling of views instead of simply allowing the option to do it (or the option to forget to do it and have problems; see #955 fixing it). If there is also a better recycling policy behind, we will only notice it by start using them.

Nevertheless, there are other important reasons to embrace this change:

The GridView widget currently in use is a mess; it's not fully implemented in all the Android versions (even in different 4,x versions). This fact already caused crashes in the past, and is still enforcing some undesired graphical effect in older 4.x devices (my lips are sealed). Using the new Grid element in Android Support Library we will have a single and complete implementation for all our grids.
Both components are needed if we want to take advantage of new elements for cool animations in Material Design.

Sure of interest for @tobiasKaminsky , @jancborchardt , and everybody is welcome.

Just please, remember, that #692 should be done first (or at least the part of the build target).

Details

Branch

Implementation will be done on feature branch https://github.com/owncloud/android/tree/984_recycler_view

Issues

Issues referenced/mentioned in this issue and their current status:

[x] #955 - MERGED - HOTFIX: prevent OOM when scrolling Thumbnails
[x] #692 - MERGED - Update to Material design: add OC THEME based on Material Design THEME
[x] #1090 - MERGED - Material buttons and checkboxes
[x] #1076 - MERGED - Material Design
[x] #1100 - MERGED - Material FAB with speed dial implementation
[x] #1473 - MERGED - Move to compile target Marshmallow (Android 6 - v23) - rebased without FAB
[x] #1559 - TO-REVIEW - Toolbar, new Drawer and new Account Manager
TODOs
[x] Bump support lib to latest version - #1557
[ ] Add recycler view dependency for gradle and ant
[ ] implement recycler view for list and grid

masensio commented 9 years ago

Grid lists and lists from Material Design http://www.google.com/design/spec/components/grid-lists.html# http://www.google.com/design/spec/components/lists.html

ddijak commented 9 years ago

why just not use recycler view and LayoutManager to show list or Grid? Plus its ViewHolder implementation would save a lot of resources, because currently what we have implemented as ViewHolder in FileListListAdapter is laughable.

Kernald commented 9 years ago

@skymania actualy, that's the whole point of this issue.

ddijak commented 9 years ago

I hope so.

jancborchardt commented 9 years ago

@skymania since it sounds like you’re experienced, maybe you’re interested in contributing? :) I’m sure if you need any help, @davivel @tobiasKaminsky @AndyScherzinger can help you.

AndyScherzinger commented 9 years ago

Absolutely! :) The material_toolbar branch is probably a good starting point since it contains most of the material implementation and the latest AppCompat release.

ddijak commented 9 years ago

I have implemented toolbar, recycler view and UIL. Currently Im working on navigation drawer but time is not currently on my side so I have no idea when I will have the time to proceed with material implementations. I have to upload all the changes I made on github tho.

jancborchardt commented 9 years ago

@skymania please do a pull request even when it's work in progress so we can already review. If a pull request is too big, risk is high it's very difficult to merge.

AndyScherzinger commented 9 years ago

@ all how should we all move forward with this overall topic, since we now have this, #1076 and #1090 and @skymania provides the next implementation (which is fine with me) and I provided the action bar variant. So imho we need some kind of coordination to not implement everything twice :)

jancborchardt commented 9 years ago

I’d say we should wait for #1076 to be QA'd and merged – which will hopefully happen soon. Then #1090 shortly after (we need it in the next cycle @davivel).

Then after that we can build on top of that. Otherwise it just becomes unmanageable. There’s no reason to cram all of these improvements in such a short amount of time now. ;)

AndyScherzinger commented 9 years ago

Sounds reasonable @jancborchardt - thanks for the clarification :)

ddijak commented 8 years ago

Finally found some time and I am currently using this for my owncloud server. Still in development tho. Made huge code optimizations so everything is running smooth as it should be.

AndyScherzinger commented 8 years ago

@skymania did you rewrite one of the material branches of the ownCloud Android app?! :question: @jancborchardt I really do like the way how @skymania is visualizing the link/download with the round icon :+1: more colors/contrast and a bold style (material style)

ddijak commented 8 years ago

the master one. 1.8.0

AndyScherzinger commented 8 years ago

hmm, okay. The fab implementation would have been on the material_fab branch already ;)

AndyScherzinger commented 8 years ago

Any news on this matter? @skymania fancy some collaboration?

I would start working on this and will create a new branch for the recycler view implementation based on the actual master a.k.a soon to be v2.0.0 all formerly mentioned PRs and issues have been merged or are at least done from a implementation point of view only missing out on review/test/merge.

I will update the issue referencing all the issues. - DONE

Branch has been created: https://github.com/owncloud/android/tree/984_recycler_view So who every wants to work with me on this, feel free to do so either via direct commits to the branch or PRs targeting 984_recycler_view

AndyScherzinger commented 8 years ago

also looping in @malkomich since this is an issue raised in 2015 :)

ddijak commented 8 years ago

@AndyScherzinger what ever you need you can take from Cirrus. It already has a recycler view implementation.

AndyScherzinger commented 8 years ago

That would be awesome! :+1:

Two though, just to be save:

Could you sign the contributor agreement https://owncloud.org/contribute/agreement/ ? (If I re-use your implementation and not do it from scratch on my own the code couldn't be merged)
Would you fancy/do code reviews on my changes / your"copied" code?

davivel commented 8 years ago

@skymania , any chance you make a PR from your fork to ownCloud with the changes for this topic?

davivel commented 8 years ago

@AndyScherzinger , I disabled checkbox on #1559 since it's not merged yet; was a bit confusing. Hope it's OK to you.

AndyScherzinger commented 8 years ago

Sure, you mean 1559 (?) I guess (drawer) which has just been mentioned because the discussion in this issue simply mentioned drawer, so I referenced it to explicitly document that that part has already been done.

davivel commented 8 years ago

That was, I fixed the comment.

My point is, over here we reserve the word "done" for things tested and merged. In this case I had said "developed". I know, too punctilious for this time... :D

ddijak commented 8 years ago

@AndyScherzinger so I sign contribution agreement and we can start? I would love to contribute and do code reviews, but just to warn you, we will have to do shit loads of code changes to get this right :D

davivel commented 8 years ago

That's nice, @skymania .

If you had to do a summary, what would those lot of changes include?

ddijak commented 8 years ago

@davivel

implementation of recycler view
search
totally changed image loading mechanism (the current one is blocking UI)
avoiding multiple calls to notifyDataSetChanged (this is happening now for loading content at start and then for every call we get back from broadcast recevier and its blocking UI in folders with lot of contents, for example folders with lots of images)
async fill and notify adapter that the data has changed

and probably something else that I have forgot now :)

AndyScherzinger commented 8 years ago

@skymania yes that is basically it :) After signing the agreement and sending it to @karlitschek you will get the corresponding access rights to the repo and we can start working on a branch in the owncloud repo itself :)

I am fully aware that it is a huge amount of work that needs to be done, probably larger than what you had to do for cirrus since we do and (and will have) at least two more requirements for the lists:

multi select support for @tobiasKaminsky's change
header(s) and footer e.g. for the new uploads view introduced in v.2.0.0
(switch) between list/grid/grid-image item views (kind of like your implementation)

I am totally in line with your list that will need to be implemented. I also think we will need an abstract implementation for reuse throughout the lists that is able to do the things you and I mentioned.

Really looking forward to the collaboration and finally bringing RecyclerView to the app :+1:

Besides that, #1559 also changes quite a lot just to introduce a new drawer. :)

AndyScherzinger commented 8 years ago

The agreement can be found here: https://owncloud.org/contribute/agreement/ When I signed it I just sent a scan, which was totally fine :)

ddijak commented 8 years ago

@AndyScherzinger done. Lets do some damage :)

davivel commented 8 years ago

Sounds great, guys.

But please, don't forget that we need to keep an order so that the process is manageable. All those changes shouldn't go in a single branch. Each of them should go in a different branch created from master, and target master when done in separate PRs.

I'm eager to see how this progresses :)

ddijak commented 8 years ago

@davivel I'll leave making seperate branches to @AndyScherzinger, he knows how the work is done here more than I do. :)

AndyScherzinger commented 8 years ago

Sure thing, that isn't to much of an issue I can create as many branches as necessary :+1: About the splitting though, I am not sure to which extend we can split it, since a basic recycler implementation would already mean a certain amount of changes. So we could then for example open separate branches for:

rudimentary recycler view implementation (1): https://github.com/owncloud/android/tree/984_recycler_view
header/footer (based on 1)
multi select (based on 1)
optimized image loading (based on 1)
search (based on 1)

ddijak commented 8 years ago

Are there any reason the 2.0.0 version is still using ActionBar instead of Toolbar implemetation?

AndyScherzinger commented 8 years ago

Yes, that is due to the fact that #1559 hasn't yet been merged back to master and thus wasn't released yet.

ddijak commented 8 years ago

@AndyScherzinger there I made PR on the branch. Its a start so many functions will not work properly, but basic navigation is implemented. We can go on on from there.

karlitschek commented 8 years ago

@AndyScherzinger @skymania Confirmed :-)

AndyScherzinger commented 8 years ago

@skymania sound good to me! Great to have you on board! :+1:

ddijak commented 8 years ago

Thnx mate :)

AndyScherzinger commented 8 years ago

@skymania you should now have write access to the owncloud repo. I would therefor suggest that we just merge your actual PR and that you then switch to a clone of the owncloud repo branch and we simply work on that one and directly push to that one :)

ddijak commented 8 years ago

@AndyScherzinger sounds good :+1:

AndyScherzinger commented 8 years ago

@skymania I just rebased to the latest master to resolve some conflicts with the latest build changes (removal of maven support). So you need to update before doing anymore commits :)

davivel commented 7 years ago

Sorry, but we need to drop issues from milestone 2.4.0 to get it out next to OC 10 server.

This time we'll move topics to backlog instead of directly to next release, so that we can schedule 2.5.0 properly.

owncloud / android

MATERIAL DESIGN: As an OC user I want that the new list & grid components from Material Design replace the current so that performance is improved in folders with hundreds of files & images #984

Details

Branch

Issues

TODOs