Problem/Motivation
This could maybe be turned into a larger issue, but this is a start and I think at least this case can be handled with a workaround. Not really sure on the right component, that can change.
Let's just say you're using the Seven admin theme. The progress bar for batch API is rendered via Classy's progress-bar.html.twig which attaches the classy/progress library to style the progress bar appropriately.
However when you have a managed file field configured to use the progress bar instead of throbber, the markup for that progress bar comes from /core/misc/progress.js and doesn't include the classy/progress library.
Note that to test this currently you need the patch from #2587755: AJAX error when using progress bar on file field widget.
Steps to Reproduce
Setup:
- Apply the patch from #2587755: AJAX error when using progress bar on file field widget
- Increase upload_max_filesize and post_max_size in your PHP config to something big like 1G
- Install Pecl progress or APC and add
apc.rfc1867 = 1
to your PHP config (status report will tell you whether you have it set up correctly or not)
Testing:
- Clean install of Drupal 8, standard profile
- Add a file field to either article or basic page content type
- On the manage form display of the content type: change the widget's progress indicator from throbber to bar with progress meter
- Make or acquire a big file, on OS X I used
mkfile 900m big.txt
- other options here - Test uploading that large file on the node add form, it should hopefully take long enough so you can see the progress meter :)
Proposed resolution
Figure out a way to include the library as needed. One idea:
In a Classy preprocess function for the file-managed-file.html.twig, attach the library if the form widget is configured to use the progress bar rather than the throbber.
libraries-extend to the rescue!
Before:
After:
Remaining tasks
- Patch
Tests- Review
User interface changes
The progress bar will look pretty when uploading files.
API changes
TBD, should be none.
Data model changes
None
Comment | File | Size | Author |
---|---|---|---|
#9 | 2662778-after.png | 31.45 KB | star-szr |
#9 | 2662778-before.png | 29.31 KB | star-szr |
#3 | 2662778-3.patch | 419 bytes | Wim Leers |
Comments
Comment #2
droplet CreditAttribution: droplet commentedComment #3
Wim LeersSimpler: use
libraries-extend
. This is exactly what that was intended for.Comment #4
Wim LeersRelated issue added for
libraries-extend
, CR at https://www.drupal.org/node/2497313.Comment #5
star-szr@Wim Leers oh that would be great, big fan of libraries stuff as you may know :)
In manually testing it didn't seem to fix it though. Added steps to reproduce to the issue summary, I wish they were shorter but haven't found an easier way yet.
Comment #6
star-szrComment #7
Wim LeersI turned CSS aggregation off.
With patch, this is present, without it is not:
Perhaps you forgot to clear caches?
Comment #8
star-szrHmm I did clear caches several times trying to get it to work, will re-check this morning though. Thanks!
Comment #9
star-szrWell I was doing something wrong last night, not sure what. Adding before and after screenshots to the issue summary.
Since the fix is in Classy we probably can't/won't add test coverage for this. This seems a bit iffy for 8.0.x if someone is loading their own library to compensate for this in a Classy subtheme, so moving to 8.1.x.
Comment #10
swentel CreditAttribution: swentel commentedHmmm the progress bar is broken on so many levels in 8.x at this point because of #2587755: AJAX error when using progress bar on file field widget and #2662932: Fix file upload progress bar, so I think this can go in 8.0.x, because well, nobody can even use it ..
Feel free to correct me of course :)
Comment #11
star-szrThanks @swentel, that's not a bad point but I'm also concerned that progress.js could be used for other things too so still seems risky to me: I don't want to assume the core file module is the only one using this. I'll leave the version as 8.0.x for now but I still think this should only be committed to 8.1.x.
Also at this time there isn't much of a gap in the calendar between 8.0.4 (or 8.0.5) and 8.1.0 anyway, so it won't be a long wait :)
Comment #12
swentel CreditAttribution: swentel commentedHeads up, #2662932: Fix file upload progress bar is now working too - JS reviews needed there :)
Comment #13
davidhernandezI think I agree with Cottser. It is essentially a bug fix, but this not adding an insignificant amount of CSS. Probably better to be safe and release it with 8.1.
Comment #15
catchI don't have a strong opinion, but given that just committing to 8.1.x for now, we can cherry-pick back later if we need to.