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.

Sign up for GitHub

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

Jump to bottom

Add versions tab to files sidebar #18748

Merged
merged 1 commit into from
Sep 6, 2015
Merged

Add versions tab to files sidebar #18748

merged 1 commit into from
Sep 6, 2015

Conversation

PVince81
Copy link
Contributor

@PVince81 PVince81 commented Sep 1, 2015

TODO

  • sidebar tab, models, collection, view
  • fix previews
  • "revert" action
  • styling
  • pagination
  • make sure that actions update the VersionCollection so that the UI reflects the latest change
  • test mimetype preview
  • test image preview

@PVince81 PVince81 added this to the 8.2-current milestone Sep 1, 2015
@PVince81
Copy link
Contributor Author

PVince81 commented Sep 2, 2015

  • BUG: don't show versions for folders

@PVince81
Copy link
Contributor Author

PVince81 commented Sep 2, 2015

  • BUG: load on scroll doesn't work properly

@jancborchardt
Copy link
Member

Looks good, except that Versions should be the rightmost tab. Activity, Sharing, Versions, in that order. Activity open by default (unless you opened the sidebar direclty by clicking Sharing or Versions of course).

@PVince81
Copy link
Contributor Author

PVince81 commented Sep 3, 2015

Dammit... once again an order issue. This depends on the loading order of the apps.
I know you hate it that plugins can specify a sort order but I don't see how we can solve this without hard-coding id strings in core, which is even worse.

@PVince81
Copy link
Contributor Author

PVince81 commented Sep 3, 2015

General ordering problem discussed here: #18280 (comment)

@PVince81
Copy link
Contributor Author

PVince81 commented Sep 3, 2015

I will add back the "More versions" button as I won't have time to make "scroll to load more" to work properly. The tricky part here is that the scrollbar isn't on the tab but on the main container, so the events have to come from there.

@karlitschek
Copy link
Contributor

i think a more versions button is totally cool for now

@PVince81 PVince81 mentioned this pull request Sep 3, 2015
Merged
@PVince81
Copy link
Contributor Author

PVince81 commented Sep 3, 2015

I added back the "More versions..." button and fixed the spinner position.

Next up:

  • TODO: writing JS unit tests for the versions tab + collection

@PVince81 PVince81 changed the title [WIP] Add versions tab to files sidebar Add versions tab to files sidebar Sep 3, 2015
Add versions tab to files sidebar
310d797 
- move versions to a tab in the files sidebar
- added mechanism to auto-update the row in the FileList whenever values
  are set to the FileInfoModel given to the sidebar
- updated tags/favorite action to make use of that new mechanism
@scrutinizer-notifier
Copy link

A new inspection was created.

@PVince81
Copy link
Contributor Author

PVince81 commented Sep 3, 2015

I'm done with the JS unit tests.

This is ready for review @schiesbn @nickvergessen @jancborchardt @rullzer @MorrisJobke

@PVince81
Copy link
Contributor Author

PVince81 commented Sep 3, 2015

also CC @blizzz here you'll find more examples how to wire up Backbone collections/models to the view

@karlitschek
Copy link
Contributor

awesome 👍

@karlitschek
Copy link
Contributor

someone else want's to review this? I would love to get this in.
@blizzz @jancborchardt @schiesbn @nickvergessen

@MorrisJobke
Copy link
Contributor

Tested and works. Unit tests also pass locally (JS & PHP) 👍

MorrisJobke added a commit that referenced this pull request Sep 6, 2015
@MorrisJobke
Merge pull request #18748 from owncloud/files-versions-tab
f2ca0f6 
Add versions tab to files sidebar
@MorrisJobke MorrisJobke merged commit f2ca0f6 into master Sep 6, 2015
@MorrisJobke MorrisJobke deleted the files-versions-tab branch September 6, 2015 22:53
MorrisJobke added a commit that referenced this pull request Sep 7, 2015
@MorrisJobke
Remove unneeded include
4251dce 
* fixes #18867
* ref #18748
@MorrisJobke MorrisJobke mentioned this pull request Sep 7, 2015
Merged
@lock lock bot locked as resolved and limited conversation to collaborators Aug 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
3 - To Review design enhancement
Projects
None yet
Milestone
8.2
Development

Successfully merging this pull request may close these issues.
5 participants
@PVince81 @jancborchardt @karlitschek @scrutinizer-notifier @MorrisJobke