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

feat: data preview on cell hover #1983

Draft
wants to merge 4 commits into
base: alpha
Choose a base branch
from

Conversation

404-html
Copy link
Member

@404-html 404-html commented Jan 4, 2022

New Pull Request Checklist

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

@parse-github-assistant
Copy link

parse-github-assistant bot commented Jan 4, 2022

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.

@404-html 404-html changed the title Data preview on cell hover feat: data preview on cell hover Jan 4, 2022
@mtrezza
Copy link
Member

mtrezza commented Jan 6, 2022

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.

@404-html
Copy link
Member Author

404-html commented Jan 6, 2022

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.

@mtrezza
Copy link
Member

mtrezza commented Jan 6, 2022

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

@404-html
Copy link
Member Author

404-html commented Jan 7, 2022

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.

@mtrezza
Copy link
Member

mtrezza commented Jan 8, 2022

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.

@404-html
Copy link
Member Author

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.

@mtrezza
Copy link
Member

mtrezza commented Jan 12, 2022

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.

@404-html 404-html mentioned this pull request Jan 15, 2022
3 tasks
@404-html
Copy link
Member Author

Sure, raised: #2002 ✅ Thanks.

@Moumouls
Copy link
Member

Moumouls commented May 7, 2022

@mtrezza @404-html using image display in an <iframe> HTML tags could help to prevent security issues ? Then the loaded resource from an external source will not have access to the Dashboard context.

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

@mtrezza
Copy link
Member

mtrezza commented May 7, 2022

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 #2002 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants