-
-
Notifications
You must be signed in to change notification settings - Fork 355
[Dropzone] Improve display with multiple files #2704
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
base: 2.x
Are you sure you want to change the base?
Conversation
📊 Packages dist files size differenceThanks for the PR! Here is the difference in size of the packages dist files between the base branch and the PR.
|
79f865d
to
8e396b0
Compare
Not seeing the images :/ |
<div class="dropzone-placeholder" data-symfony--ux-dropzone--dropzone-target="placeholder"> | ||
{%- if attr.placeholder is defined and attr.placeholder is not none -%} | ||
{{- translation_domain is same as(false) ? attr.placeholder : attr.placeholder|trans({}, translation_domain) -}} | ||
{%- endif -%} | ||
</div> | ||
|
||
<button class="dropzone-preview-button" type="button" data-symfony--ux-dropzone--dropzone-target="previewClearButton"></button> |
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.
This is problematic, as people would have a new button that appears on their forms / UI..
I think we do need to create a new one
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.
But this is the same actual "clear" button, just move it outside the "preview" div.
Behavior is the same, you have a small X "close" button (but look nicer with a svg) at the top right only when you have file in the dropzone.
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.
I understand and I'd be happy to offer a nicer button believe me (you would need to change it's class tbf but that is a detail).
What i'm refering to is big BC break. If they used and customized the CSS, you are changing the HTML of their form doing so, and this can break websites.
So the idea would be to either
- offer a new theme
- release this new feature in new major version
src/Dropzone/assets/src/style.css
Outdated
--dropzone-file-svg: url('data:image/svg+xml,<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24"><path fill="none" stroke="currentColor" stroke-linejoin="round" stroke-width="2" d="M10 3v4a1 1 0 0 1-1 1H5m14-4v16a1 1 0 0 1-1 1H6a1 1 0 0 1-1-1V7.914a1 1 0 0 1 .293-.707l3.914-3.914A1 1 0 0 1 9.914 3H18a1 1 0 0 1 1 1Z"/></svg>'); | ||
--dropzone-close-svg: url('data:image/svg+xml,<svg xmlns="http://www.w3.org/2000/svg" width="1em" height="1em" viewBox="0 0 24 24"><path fill="none" stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="m15 9l-6 6m0-6l6 6m6-3a9 9 0 1 1-18 0a9 9 0 0 1 18 0"/></svg>'); |
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.
I think this would be better in the theme, not sure everyone want this
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.
Not sure to understand; actually the "clear" button is a simple content:-"x"
in CSS (not sure everyone want this :p), IMO, SVG is a better solution, and with the CSS variable you can easily custom it.
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.
Using SVGs as data URLs (data:image/svg+xml;base64,...) is often restricted in enterprise environments. This is mainly due to security policies like CSP (Content Security Policy) that block data: schemes or inline content. Encoded SVGs can also contain JavaScript or CSS, which raises concerns about XSS vulnerabilities. Additionally, data URLs bypass HTTP-level caching and CDN optimization, making them less efficient for large-scale or public-facing applications.
For better compatibility and maintainability, it’s generally recommended to use static SVG files served over HTTP, inline elements (without embedded scripts or styles), or SVG sprites.
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.
I moved svg to js and html files respectively
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.
(sorry misslicked on approve)
8e396b0
to
8444bbe
Compare
8444bbe
to
35c9e51
Compare
.dropzone-on-drag-leave { | ||
background-color: var(--dropzone-background); | ||
transition: 0.3s; |
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.
If this is the only reason to add the 2 classes, maybe it's not 100% required, wdyt ? :)
(the same thing can be done with :has
selectors)
@@ -1,24 +1,17 @@ | |||
{% block dropzone_widget -%} | |||
{%- set dataController = (attr['data-controller']|default('') ~ ' symfony--ux-dropzone--dropzone')|trim -%} | |||
{%- set attr = attr|merge({'data-controller': '', class: (attr.class|default('') ~ ' dropzone-input')|trim}) -%} | |||
{%- set dataController = (attr['data-controller']|default('') ~ ' symfony--ux-dropzone--dropzone')|trim -%} |
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 revert the indentation as it was ? 😄
It's to avoid unnecessary changes, and ease the review / maintain process (now and after i mean)
this._populateImagePreview(file, imagePreviewElement); | ||
} else { | ||
const noPreviewSvg = | ||
'<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24"><path fill="currentColor" d="M14 11a3 3 0 0 1-3-3V4H7a2 2 0 0 0-2 2v13a2 2 0 0 0 2 2h9a2 2 0 0 0 2-2v-8zm-2-3a2 2 0 0 0 2 2h3.59L12 4.41zM7 3h5l7 7v9a3 3 0 0 1-3 3H7a3 3 0 0 1-3-3V6a3 3 0 0 1 3-3"/></svg>'; |
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.
Let's keep SVG in HTML :)
That way use can change them easily with their own form theme
<div data-symfony--ux-dropzone--dropzone-target="previewFilename" class="dropzone-preview-filename"></div> | ||
</div> | ||
</div></div></div></form> | ||
'<form name="form" method="post" enctype="multipart/form-data"><div id="form"><div><label for="form_photo" class="required">Photo</label><div class="dropzone-container" data-controller="mydropzone symfony--ux-dropzone--dropzone"> |
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.
This is the proof we cannot do these changes without a dedicated form theme and/or controller js : the structural changes in the HTML are going to break layout/tests of people currently using this component
Actually, the dropzone will only show one file, even if it allows multiple files to be uploaded. This makes it difficult to see which file has been added to the dropzone.
This PR adds a preview of multiple files (not just the first). There are several previous issues or PRs starting to achieve this (#2642.) but with a different approach.
If the dropzone is small or there are many files, they will be displayed on a multiline grid.

If the file is NOT an image, a simple file icon will be displayed (to keep the grid aligned and show all file names).

The style has also been modernised (effect on hover or drop, new icon for clear button) and some CSS variables added to the style file.