-
-
Notifications
You must be signed in to change notification settings - Fork 90
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
Added sidebar to the viewer #418
Conversation
Signed-off-by: Arne Hamann <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice! I didn't know it was possible to embed the Files sidebar!
Just a few minor changes and we can merge.
Oh and also, we get 3 Javascript errors on page load:
Uncaught TypeError: can't access property "extend", OCA.Files.DetailTabView is undefined
Uncaught TypeError: can't access property "Client", t.Files is undefined
Uncaught TypeError: can't access property "extend", OCA.Files.DetailTabView is undefined
src/photosController.js
Outdated
@@ -205,7 +207,7 @@ PhotosController.prototype = { | |||
iconUrl = _app.getImageIconUrl(); | |||
} | |||
var label = cluster.getChildCount(); | |||
if( availZoomLevels == 0 && label > 1){ | |||
if( availZoomLevels == 0 && label > 1){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also remove those changes on trailing white spaces? Or maybe fix the code style in this file?
This line could be:
if (availZoomLevels === 0 && label > 1) {
I thing the errors are a problem we are not going to solve now. They happen for the photos app, too. I think it is related to different tabs which are not yet migrated. |
Ok so ready to merge after having removed the |
Signed-off-by: Arne Hamann <[email protected]>
Yep, you can ignore those :) |
Signed-off-by: Arne Hamann [email protected]