-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
base: alpha
Are you sure you want to change the base?
Conversation
Thanks for opening this pull request!
|
Good idea! Some thoughts:
Executing a URL request on hover may not be desired and could even pose security issues, e.g.:
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. |
Thanks @mtrezza, that's a valid concern. How about making hover-image-fetch-preview configurable in Dashboard configuration and turning it off by default? |
And these URLs are not related to Parse Server managed storage? |
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. |
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 I hope that's clear. Let me know please if you think that still needs raising an issue. |
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. |
Sure, raised: #2002 ✅ Thanks. |
@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 |
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). |
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.
Related issue: none
Approach
Preview handlers for different data types. See
StringDataHandler
,PointerDataHandler
orObjectDataHandler
.TODOs before merging
Popover
still behaves the same after adding logic for fixing position; testModal
component