Conversation
Contributor
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||
enyo
added this pull request to stack #2364
September 12, 2026 17:56
enyo
force-pushed
the
inject-css
branch
3 times, most recently
from
September 13, 2026 10:45
fc9e68b to
9179c6d
Compare
Setting injectStyles makes Dropzone insert its stylesheet into the document itself, so there is no link tag to remember and no path to keep in step with the package. It takes which one: false add nothing, the default true or "full" dropzone.css, the ready-to-go styling "basic" basic.css, layout only They are alternatives rather than layers, which is the reason the option names them rather than counting them: basic.css is not a subset of dropzone.css. Twenty of its thirty-five declarations are absent from the full sheet, including `position: relative` on .dropzone -- the full theme positions against .dz-preview instead -- along with the progress bar and the success and error marks. Picking one excludes the other, so a name like "all" would have been a lie. The stylesheets are carried as strings rather than imported for their side effect, so nothing reaches the document unless the option asks for it. It is inserted once per page however many dropzones exist, and prepended to head rather than appended. Appending would put it after a stylesheet the page already links, so switching the option on would quietly override styling that used to work. Prepending means page rules keep winning on equal specificity. The style element is tagged with which variant went in, so a second dropzone asking for the other can be seen to have been ignored rather than silently doubling up. It runs before the fallback check, because the fallback form is styled by the same stylesheet. Defaults to false, and this is the tradeoff worth stating plainly: both stylesheets travel inside the JavaScript bundle whether or not the option is on, because a runtime condition cannot be tree-shaken. That is 1.4 kB gzipped, on a bundle that was 11.8 kB. Anyone importing the CSS through a bundler should keep doing that; this is for the people who would otherwise ship no stylesheet at all. The end-to-end tests load pages with no stylesheet link whatsoever and assert computed values that only one of the two sheets can produce, so they prove the right CSS survived bundling into the standalone file rather than merely that a style element appeared.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #2362. Setting
injectStylesmakes Dropzone insertdropzone.cssinto the document itself — no<link>to remember, no path to keep in step with the package.The CSS is carried as a string rather than imported for its side effect, so nothing reaches the document unless the option asks for it.
Two decisions worth reviewing
It is prepended to
<head>, not appended. Appending would place it after a stylesheet the page already links, so turning the option on would quietly override styling that used to work. Prepending means your rules keep winning on equal specificity, with no!important. There is a test for the ordering, because it is the kind of thing a later refactor silently reverses.It runs before the fallback check, since the fallback form is styled by the same stylesheet.
The cost, stated plainly
injectStylesdefaults tofalse, and this is why: the stylesheet travels inside the JavaScript bundle whether or not the option is switched on, because a runtime condition cannot be tree-shaken.dropzone-min.jsThat is +1.3 kB gzipped, about 11%, paid by everyone. Anyone importing the CSS through a bundler should keep doing that — it stays the smaller option, and the docs say so. This is for the people who would otherwise ship no stylesheet at all.
If that cost turns out to bother you, the alternative is a separate entry point (
dropzone/with-styles) that only its importers pay for — but that is an import, not an option, which is not what you asked for.Tests
7 unit tests and 2 end-to-end. The end-to-end ones matter more than usual: the page they load links no stylesheet at all, and the assertion is a computed
min-heightof150px. That proves the CSS survived bundling into the standalone file and is actually applying — not merely that a<style>element appeared.259 unit tests and 5 end-to-end specs pass overall. The docs are updated in both places: a row in the options table, and a section under Installation → CSS that includes the size tradeoff.
A fix that belongs to #2362
While building this I found #2362's CI failing on
format:checkforsrc/dropzone.css. Cause:oxfmtruns with--ignore-path .gitignore, and the stalesrc/.gitignoreI deleted in that pull request had been hiding the file from the formatter as well as from git. Fixed on that branch, not this one, and this branch was rebased onto it.