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

Bugs/features after the new sidebar #17737

Closed
12 of 14 tasks
skjnldsv opened this issue Oct 29, 2019 · 12 comments
Closed
12 of 14 tasks

Bugs/features after the new sidebar #17737

skjnldsv opened this issue Oct 29, 2019 · 12 comments

Comments

@skjnldsv
Copy link
Member

skjnldsv commented Oct 29, 2019

After #15719

parity

cleanup

bugs

@skjnldsv skjnldsv added bug enhancement design Design, UI, UX, etc. 1. to develop Accepted and waiting to be taken care of high feature: files regression labels Oct 29, 2019
@skjnldsv skjnldsv added this to the Nextcloud 18 milestone Oct 29, 2019
@skjnldsv skjnldsv removed the bug label Oct 29, 2019
@juliusknorr
Copy link
Member

Added:

  • The currently selected file is not highlighted in the file list anymore
  • Sidebar preview plugins (e.g. from files_pdfviewer) are no longer working

@skjnldsv
Copy link
Member Author

skjnldsv commented Dec 5, 2019

  • Sidebar preview plugins (e.g. from files_pdfviewer) are no longer working

this is done on purpose.
they should comply to proper preview generator :)

Because loading big files directly is not something we want (txt, pdf...)
It's too heavy (and probably not ok security wise)!

@juliusknorr
Copy link
Member

I think that is a regression that people will complain about a lot. I also fear that there is no real way to have a proper preview of markdown documents generated as an image with just PHP and we should have nice previews for documents of the text app for example.

@juliusknorr
Copy link
Member

Also plaintext previews look really bad imo:
image

@skjnldsv
Copy link
Member Author

skjnldsv commented Dec 5, 2019

Pdf can be rendered with imagick.
This is the same as videos. We could inject lots of scripts into the sidebar to fake a real preview, but it's:

  1. heavy to load the script
  2. heavy to load the file
  3. complexify the whole thing a lot and is much sloppier
  4. can lead to security issues if we load a file that contains a security hack or something (pdf xss...)

@szaimen
Copy link
Contributor

szaimen commented Dec 5, 2019

Pdf can be rendered with imagick. This is the same as videos.

Aren't there problems with imagick?
#13099

Apart from that, I agree with @skjnldsv.

@wiswedel
Copy link
Contributor

Cannot open the sidebar from the share icon breadcrumbs

This is checked as resolved, but still happens in beta2 - has the fix been merged just too recently or is something still wrong?

@skjnldsv
Copy link
Member Author

skjnldsv commented Dec 16, 2019

This is checked as resolved, but still happens in beta2 - has the fix been merged just too recently or is something still wrong?

fix was merged but another pr broke it.
#18425

@skjnldsv skjnldsv added 2. developing Work in progress and removed 1. to develop Accepted and waiting to be taken care of labels Dec 16, 2019
@jancborchardt
Copy link
Member

What about the The tags should not me shown if none are set, it seems to be blocked by "differentiate focus and focus-keyboard on actions" nextcloud-libraries/nextcloud-vue#704

Now that @jospoortvliet is taking screenshots, the "Collaborative tags" box is in all the screenshots and it doesn’t look good. cc @skjnldsv :)

@skjnldsv
Copy link
Member Author

skjnldsv commented Jan 2, 2020

What about the The tags should not me shown if none are set, it seems to be blocked by "differentiate focus and focus-keyboard on actions" nextcloud/nextcloud-vue#704

No it's just a matter of adding a toggle for it :(
No time yet

@rullzer rullzer modified the milestones: Nextcloud 18.0.2, Nextcloud 18.0.3 Mar 11, 2020
@juliusknorr
Copy link
Member

The tags should not me shown if none are set, we should add that to the actions menu on the sidebar

Already fixed by #18732

@juliusknorr
Copy link
Member

Closing since all server related ones are fixed and only nextcloud-libraries/nextcloud-vue#704 is remaining to be fixed in the components.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

7 participants