parse-dashboard icon indicating copy to clipboard operation
parse-dashboard copied to clipboard

feat: data preview on cell hover

Open 404-html opened this issue 3 years ago • 11 comments

New Pull Request Checklist

  • [x] I am not disclosing a vulnerability.
  • [ ] I am creating this PR in reference to an issue.

Issue Description

Let's move Parse Dashboard to the next level. This PR is about showing data preview when cell is hovered for a while. Currently only for String type of data but it is designed to be easily extendable for other data types. If string is a URL the algorithm checks if it's an image, and if so - it shows it.

preview-on-hover

Related issue: none

Approach

Preview handlers for different data types. See StringDataHandler, PointerDataHandler or ObjectDataHandler.

TODOs before merging

  • [ ] Add tests
  • [ ] Add changes to documentation (guides, repository pages, in-code descriptions)
  • [ ] A changelog entry is created automatically using the pull request title (do not manually add a changelog entry)
  • [ ] fix popover not appearing after selecting the cell
  • [ ] make sure Popover still behaves the same after adding logic for fixing position; test Modal component

404-html avatar Jan 04 '22 22:01 404-html

Thanks for opening this pull request!

  • ❌ Please check all required checkboxes at the top, otherwise your pull request will be closed.

  • ⚠️ Remember that a security vulnerability must only be reported confidentially, see our Security Policy. If you are not sure whether the issue is a security vulnerability, the safest way is to treat it as such and submit it confidentially to us for evaluation.

Good idea! Some thoughts:

If string is a URL the algorithm checks if it's an image, and if so - it shows it.

Executing a URL request on hover may not be desired and could even pose security issues, e.g.:

  • an app user stores a malicious URL in their user profile; a dashboard users hovers over it an executes a call to that URL
  • a dashboard user may not want to download a large image file

I would rather see a preview exclusively for fields of type File and with a button that has to be explicitly clicked to download and display the file (image, video, pdf, etc) in a pop-up dialog. There is already a download button. I would also suggest that the preview dialog has to be explicitly closed (not on hover-out), for easy navigation and because it has also been opened with a click.

mtrezza avatar Jan 06 '22 12:01 mtrezza

Thanks @mtrezza, that's a valid concern. How about making hover-image-fetch-preview configurable in Dashboard configuration and turning it off by default? I found myself quite often in a situation in which to preview an image stored in database as an url I have to perform like 7 clicks, which is quite frustrating when there's many images. That's why I'm coming with this handy proposition.

404-html avatar Jan 06 '22 20:01 404-html

And these URLs are not related to Parse Server managed storage?

mtrezza avatar Jan 06 '22 21:01 mtrezza

Correct. In the era of cloud providers I think there's a trending practice to store media files on cloud buckets instead of the database because it's basically cheaper.

404-html avatar Jan 07 '22 09:01 404-html

Correct. In the era of cloud providers I think there's a trending practice to store media files on cloud buckets instead of the database because it's basically cheaper.

Parse Server does support this via storage adapters. The files are then stored e.g. in Amazon S3.

My question is this: should the feature you are suggesting be related to Parse Files which are stored in these storage services and managed by Parse Server via its storage adapters? Or is this about a preview for a URL in a String field where the URL is storage agnostic, regardless of where it points to and whether it's managed by Parse Server. If related to Parse Files, the feature would be attached to the Parse File field type, if agnostic, it would probably be attached to the String field.

This is something we can better discuss in an issue rather than in this PR. I suggest you open an issue for this to give it more visibility and make it easier for others to join the discussion.

mtrezza avatar Jan 08 '22 22:01 mtrezza

should the feature you are suggesting be related to Parse Files which are stored in these storage services and managed by Parse Server via its storage adapters? Or is this about a preview for a URL in a String field where the URL is storage agnostic, regardless of where it points to and whether it's managed by Parse Server.

I think the answer here is: both. That's because I designed data preview to process various data types differently. Currently I provided a handler for String type of data (and handled strings that looks like an URL as it fulfill my needs) and left TODOs for other types. Anyway it can be easily extended to handle File type of data in a desired way. So there's basically no conflict here and these two different cases can be distinguished. If community will find such quick data preview useful then maybe handlers for other data types and cases will be created.

I hope that's clear. Let me know please if you think that still needs raising an issue.

404-html avatar Jan 12 '22 20:01 404-html

Could you please open an issue and link to this PR anyway, for the aforementioned reasons. Just copy/paste some of your comments so we can continue the discussion there.

mtrezza avatar Jan 12 '22 20:01 mtrezza

Sure, raised: https://github.com/parse-community/parse-dashboard/issues/2002 ✅ Thanks.

404-html avatar Jan 15 '22 10:01 404-html

@mtrezza @404-html using image display in an

https://stackoverflow.com/questions/4149469/displaying-image-in-iframe

Moumouls avatar May 07 '22 08:05 Moumouls

The basic issue is that a request to a user-injected URL may be executed involuntarily; that alone opens up a world of attack vectors. Discussed in https://github.com/parse-community/parse-dashboard/issues/2002#issuecomment-1013672560.

mtrezza avatar May 07 '22 08:05 mtrezza