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

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

Open
8 of 10 tasks
davivel opened this issue May 21, 2015 · 42 comments

Comments

@davivel
Copy link
Contributor

davivel commented May 21, 2015

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:

TODOs

  • Bump support lib to latest version - Bump support libs to 23.3 #1557
  • Add recycler view dependency for gradle and ant
  • implement recycler view for list and grid
@masensio
Copy link

masensio commented Jun 3, 2015

@ddijak
Copy link

ddijak commented Aug 8, 2015

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
Copy link
Contributor

Kernald commented Aug 8, 2015

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

@ddijak
Copy link

ddijak commented Aug 8, 2015

I hope so.

@jancborchardt
Copy link
Member

@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
Copy link
Contributor

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
Copy link

ddijak commented Aug 9, 2015

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
Copy link
Member

@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
Copy link
Contributor

@ 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
Copy link
Member

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
Copy link
Contributor

Sounds reasonable @jancborchardt - thanks for the clarification :)

@ddijak
Copy link

ddijak commented Nov 26, 2015

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
Copy link
Contributor

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

@ddijak
Copy link

ddijak commented Nov 26, 2015

the master one. 1.8.0

@AndyScherzinger
Copy link
Contributor

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

@AndyScherzinger
Copy link
Contributor

AndyScherzinger commented Apr 19, 2016

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 AndyScherzinger self-assigned this Apr 19, 2016
@AndyScherzinger
Copy link
Contributor

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

@ddijak
Copy link

ddijak commented Apr 19, 2016

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

@AndyScherzinger
Copy link
Contributor

AndyScherzinger commented Apr 19, 2016

That would be awesome! 👍

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
Copy link
Contributor Author

davivel commented Apr 19, 2016

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

@davivel
Copy link
Contributor Author

davivel commented Apr 19, 2016

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

@AndyScherzinger
Copy link
Contributor

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
Copy link
Contributor Author

davivel commented Apr 19, 2016

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

@AndyScherzinger
Copy link
Contributor

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
Copy link

ddijak commented Apr 25, 2016

@AndyScherzinger done.
Lets do some damage :)

@davivel
Copy link
Contributor Author

davivel commented Apr 25, 2016

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
Copy link

ddijak commented Apr 25, 2016

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

@AndyScherzinger
Copy link
Contributor

AndyScherzinger commented Apr 25, 2016

Sure thing, that isn't to much of an issue I can create as many branches as necessary 👍
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:

@ddijak
Copy link

ddijak commented Apr 25, 2016

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

@AndyScherzinger
Copy link
Contributor

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

@ddijak
Copy link

ddijak commented Apr 25, 2016

@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
Copy link

@AndyScherzinger @Skymania Confirmed :-)

@AndyScherzinger
Copy link
Contributor

@Skymania sound good to me! Great to have you on board! 👍

@ddijak
Copy link

ddijak commented Apr 25, 2016

Thnx mate :)

@AndyScherzinger
Copy link
Contributor

@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
Copy link

ddijak commented Apr 25, 2016

@AndyScherzinger sounds good 👍

@AndyScherzinger
Copy link
Contributor

@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 davivel added this to the backlog milestone Jul 14, 2016
@davivel davivel modified the milestones: 2.3.0, backlog Aug 12, 2016
@davivel davivel modified the milestones: 2.4.0, 2.3.0 Mar 9, 2017
@davivel davivel modified the milestones: backlog, 2.4.0 Apr 11, 2017
@davivel
Copy link
Contributor Author

davivel commented Apr 11, 2017

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.

@jesmrec jesmrec removed the delayed label Mar 27, 2019
@AndyScherzinger AndyScherzinger removed their assignment Apr 1, 2022
@JuancaG05 JuancaG05 removed this from the backlog milestone May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants