Problem/Motivation
This issue is postponed on all must-haves from #2834729: [META] Roadmap to stabilize Media Library.
Once the roadmap for the Media Library is complete, we'll need signoffs on the final state as well as a patch to actually mark the module stable.
Proposed resolution
- Add to or update
MAINTAINERS.txt
entry for the module as needed (@phenaproxima accepted maintainership in #15, @seanB accepted in #17). - Remove the experimental module designation from
media_library.info.yml
. - Remove the
@internal
from Media Library APIs intended to be public. - Replace the
@internal
text for APIs that are intended to remain internal even after the module is stable with explanations of why the code in question is internal (e.g., specific categories from https://www.drupal.org/core/d8-bc-policy, etc.) - Copy CSS and templates to Stable or Classy as appropriate.
- Address the following other markup and CSS issues:
Remaining tasks
Release management checklist
- ✅ Security: Known public and private security issues are addressed, including #3063216: Inaccessible Media entities still have rows generated for them in the Media Library view, containing form wrapper markup *and* the media label, #2969678: Integrate Media Library with Content Moderation, etc.
- ✅ Data integrity and upgrade path: Known issue addressed with #2981105: Media Library should not modify the media view
- ✅ Impact on stable functionality
Integrations
- ✅ CKEditor: Addressed in the must-haves of #2801307: [META] Support WYSIWYG embedding of media entities.
- ✅ REST and JSON:API: Handled by Media itself; Media Library is specifically a UI-facing... UI.
- ✅ Content Moderation: Addressed in #2969678: Integrate Media Library with Content Moderation.
- ✅ Multilingual functionality: Addressed in #3081587: Multilingual content is shown double in the media library view and #3087126: Media Library "select all" checkbox does not have proper margin in RTL styling (included in this patch)
- Other areas?
- Maintenance and technical debt
- ✅ Active maintainers listed with their agreement to maintainer roles documented
- ✅ Issue queue review
We've done lots of triage with the maintainers, accessibility reviewers, etc. There are between 20-25 major issues outstanding, and no criticals. About half of the major issues are further accessibility work for the module, some of which will be required before the module is added to the Standard install profile, as described in #2825215: Media initiative: Roadmap. There has been really great progress to reduce the major issue count over the past two months and to improve the accessibility as much as possible before we mark it stable.
Great work!
- API definition and BC policy (mostly ✅, see #39; one remaining fix needed in #3087456: Move some representational classes in Media Library to Classy and others to Seven and/or Claro, as appropriate)
- Core gates
- ✅ Usability: All must-haves identified during numerous usability reviews were addressed in #2834729: [META] Roadmap to stabilize Media Library
- ✅ Accessibility
- Testing: ✅ for must-haves.
Should-have issues to finish before 8.8.0 if possible: - ✅ Documentation
- Performance
User interface changes
API changes
Data model changes
Release notes snippet
Media Library provides content editors and site builders an interface to visually browse and manage media in their site. It also provides an intuitive modal dialog for reusing media in entity reference fields and text editors. Users with appropriate access can also add new media from directly within the library.
This module was introduced as an experimental core module in Drupal 8.6.0, but is now stable and ready for production use! For sites using the experimental module prior to 8.8.0 there are some important changes:
- All of Media Library's styling and associated CSS classes were moved into Classy and Seven. Classy provides a very minimal amount of basic layout for the media library; Seven provides a more complete experience. If you are not using Seven to display the media library, you may need to add code to your theme to ensure that the moved CSS classes are applied to the media library in the correct places. See https://www.drupal.org/node/3089245 and https://www.drupal.org/node/3089300 for more information, and
core/themes/seven/seven.theme
for examples of how to apply the required CSS classes in your theme. - The "add forms" provided by the Media Library module now have two different form IDs. Previously, both shared the same form ID (
media_library_add_form
). This is now their shared base form ID. Their individual form IDs are nowmedia_library_add_form_upload
andmedia_library_add_form_oembed
. Any code altering either of these forms may be adjusted, and any custom form extending\Drupal\media_library\Form\AddFormBase
will need to be changed as well. See https://www.drupal.org/node/3089217 for more information. - The media library's administrative grid interface is no longer exposed at
/admin/content/media
. That path now shows the administrative table listing of media items, as it does without Media Library installed. The grid interface is linked from there, and exposed at/admin/content/media-grid
. An update path for this is provided; no further action is needed from existing sites.
Comment | File | Size | Author |
---|---|---|---|
#121 | interdiff-3082690-96-121.txt | 316 bytes | phenaproxima |
#121 | 3082690-121-D9.patch | 118.03 KB | phenaproxima |
#98 | 3082690-96-D9.patch | 118.13 KB | JeroenT |
#96 | 3082690-96.patch | 118.02 KB | oknate |
| |||
#96 | interdiff--3082690--87-96.txt | 281 bytes | oknate |
Comments
Comment #2
xjmComment #3
xjmComment #4
xjmComment #5
xjmComment #6
phenaproximaDiscussed with @seanB about the
@internal
vs.@api
stuff.Short answer: everything should be
@internal
except for the following:These two things were meant to be implemented and/or extended by external code, but they were marked internal because Media Library is (currently) experimental. When we're stable, we're comfortable explicitly opening these up to extension.
It seems reasonable to go stable with the smallest possible "API surface", and open things up later as needed. It's a lot easier to do that than to open everything up at once and have to rope things off later.
Comment #7
phenaproximaHere is an initial patch which updates MAINTAINERS.txt, the info file, and all PHP classes with the agreed-upon
@internal
or@api
designation.Comment #8
phenaproximaBecause it was "virtually committed" (i.e., marked postponed), I'm rolling #3035994: Add a Media Library views template to Seven theme to change to order of the views header and exposed filters into this patch, since no further work is expected on that one.
Comment #14
phenaproximaTransferring credit from #3035994: Add a Media Library views template to Seven theme to change to order of the views header and exposed filters, since that issue is now concluded.
Comment #15
phenaproximaCurrently, the patch lists @seanB and myself as maintainers of Media Library. I can't speak for @seanB, but to fulfill governance requirements: I formally accept maintainership of this module.
Comment #16
phenaproximaComment #17
seanBI too, formally accept maintainership of the module.
Comment #18
phenaproximaThis includes the changes from #3038489-33: Add a Media library views template to Seven theme to add wrapper around the grid rows, which was reviewed and approved by @lauriii today in a Zoom call. One to go!
Comment #20
phenaproximaTransferring credit from #3038489: Add a Media library views template to Seven theme to add wrapper around the grid rows.
Comment #21
webchickTo help expedite the final reviews of this patch, once we get to the finish line, would you be able to please also upload a patch that is only the changes not already reviewed/"committed" by other issues? That would expedite the final reviews to ensure we're only focusing on new stuff.
Comment #22
xjmAye, having both an "interdiff" from those and a complete patch would be handy.
Comment #23
oknateInterdiff from #8 to #18.
Comment #24
oknate#3038489: Add a Media library views template to Seven theme to add wrapper around the grid rows has landed. So my interdiff posted in #23 might be off. But for a good reason!
Comment #25
bnjmnm#18 was the first patch in this issue that tests were run on. The three fails there were all related to tests that run on modules marked stable. @phenaproxima and I worked on addressing these.
StableLibraryOverrideTest
andStableTemplateOverrideTest
due to the module's theming happening in Seven instead of Stable. Media library was added to the$<thing>toSkip
variable in both instances.DefaultConfigTest
was addressed by moving optional config from the module to the Standard profile.Comment #27
bnjmnmThis adds the changes from #3082690: Mark Media Library as a stable core module, making this ready for review.
changes-exclusive-to-this-issue_27.txt is a diff with only the changes that occurred in this issue.
Comment #28
phenaproximaMy eyes are not dry, friends. Cry if you feel it!
This has been a very, very long road for us to walk together. From the Berlin sprint of 2016, to the Barcelona sprint of 2018, to 2019 Dev Days in Cluj...and all the things we did in between. Wim said it well in his blog post about the CKEditor embedding stuff: the Media Initiative has a massive scope, and this feels like, at least to some extent, the culmination of it. There's more to do, but look what have already done!
I'm not listed officially as the initiative coordinator for Media, and that's fine with me, but it's the role I've been playing for at least the last couple of releases. I'm not even very good at herding cats; I prefer to code. But since this was the position in which I found myself, I have to say that I could NOT, ever, not even in my wildest dreams, have asked for a better set of people to labor on this thing with. You all know who you are.
So: thanks for everything! This has been, in a word, epic.
Now, that said: I'm going to take a hiatus from this initiative between 8.9 -> 9.0, when no features are being added, but I will be back in 9.0 for the final push -- migrating File and Image fields to Media. I can still review issues and do some Media stuff, and I'll be in Slack as always, but I will not be working to push the initiative forward until 9.0 is released. (Keep me in MAINTAINERS.txt.)
Let me do now what I do best: prematurely mark this issue RTBC on the assumption that tests will pass. Cheers, hugs, and victory to all!
Comment #29
phenaproximaDraft change record written: https://www.drupal.org/node/3087431
Comment #30
svettes CreditAttribution: svettes at Acquia commentedI'm not crying, YOU'RE CRYING, Adam!!!
crossing fingers, feels like a lock. You've done an amazing job leading this as far as you have, and you can be very proud of all the hard work.
Thanks Adam, and all those involved for your dedication and the *years* of work that went into this.
Me: < sings For they're some jolly good people >
Comment #33
webchickI just went through and did a thorough walkthrough of this module's functionality at #3034242-58: Hide "Save and insert" and "Additional selected media" from users by default. (As well as over and over, throughout the past couple of weeks.)
I really think this feature will be a game-changer for Drupal. It's baseline expected functionality for a content management system today, and the amazing work that's gone in over the years for usability, design, and flow has been nothing short of astounding. GREAT work, everyone!!
Product management: Signing off! *salutes*
Comment #34
effulgentsia CreditAttribution: effulgentsia at Acquia commentedI agree that this module is ready to be stable. Great work, everyone!
Comment #35
phenaproximaRefined the
@internal
stuff in the grand Layout Builder tradition, and added media_library.api.php at Wim's suggestion, to document how the module is put together.Comment #36
lauriiiAs part of my review, I realized that media library is adding a lot of classes in templates, form API and preprocess functions. We should move these to Classy: #3087456: Move some representational classes in Media Library to Classy and others to Seven and/or Claro, as appropriate.
Comment #37
Wim Leers#35: that looks great! Having worked on various parts of Media Library but definitely not all of it, I think the documentation you wrote for
media_library.api.php
captures the architecture, scope and intent clearly. It helps future Drupal core contributors understand the module, and hence improves its maintainability and debuggability. 👏👍Comment #38
xjmThanks all!
As discussed Friday, we need #3087456: Move some representational classes in Media Library to Classy and others to Seven and/or Claro, as appropriate included in-scope here. So marking NW for that and also adding it to the roadmap issue.
Comment #39
xjmCan we add this as an indented sub-section under Media? See the database API and Migrate for similar examples.
I read over all the high-level API documentation as well as the public and internal API definition as documented on individual classes, and it all looks great! Nice work.
Comment #40
xjmI've read over the module description, the module handbook page, and the
hook_help()
as well, and all look pretty good. A few small points of feedback:Are the
hook_help()
explanations regarding the original media listing view still completely accurate following the fix in #2981105: Media Library should not modify the media view?Minor point regarding:
Typically, we'd conditionally link Views UI here (either the UI itself, or the help documentation for it) if Views UI is enabled and the user has access to it.
The wording here is a little awkward:
I'd suggest instead:
The same change should be made in the last two bullets.
Comment #41
xjmComment #42
xjmI think this should also have a small release note mentioning the unusual shuffling of frontend code we did compared to most experimental modules when they become stable. (It's already tagged for it.) The release note should also link the relevant change records for those things -- maybe we should link them separately at the bottom of the main CR for this issue?
Comment #43
xjmThe accessibility gates have also been passed based on the maintainers' feedback on #2834729: [META] Roadmap to stabilize Media Library.
Comment #44
phenaproximaNeeds a reroll now that #3088476: Document Media Library's API and architecture is in. :)
Comment #45
bnjmnmRerolled.
Comment #46
phenaproximaAddressed all of @xjm's feedback in #39 and #40. Leaving NW until #3087456: Move some representational classes in Media Library to Classy and others to Seven and/or Claro, as appropriate is in.
Regarding this:
The answer is yes, they are. That issue did not make any user-facing changes.
Comment #47
phenaproximaNote that, per @lauriii in #3087456-80: Move some representational classes in Media Library to Classy and others to Seven and/or Claro, as appropriate, all of Media Library's templates should be added to Stable in this patch.
Comment #48
phenaproxima#3087456: Move some representational classes in Media Library to Classy and others to Seven and/or Claro, as appropriate has been marked fixed by @lauriii, so this needs a reroll on top of that. From here on out, we'll post two patches each time we update: the "real" patch we will commit in this issue, and a review-only ("-do-not-test.patch") patch that contains only the things which are changing in this issue.
Comment #49
phenaproximaAnd, reroll complete.
Comment #50
phenaproximaSee above.
Comment #52
phenaproximaThis should bring the tests back from the red.
Comment #53
phenaproximaMinor indentation fail. Can be fixed on commit, or in the next iteration of this patch. Whichever comes first.
Comment #54
phenaproximaNW pending small last-minute cleanup in #3087456-109: Move some representational classes in Media Library to Classy and others to Seven and/or Claro, as appropriate.
Comment #55
effulgentsia CreditAttribution: effulgentsia at Acquia commentedPart of what this patch does is moves a bunch of CSS from the module to Seven.
So I was curious how that affects what the Media Library looks like in Bartik. I created two separate fresh installs on my local machine: one with 8.8.x HEAD, one with #52 applied. For each one, I enabled Media Library, and then added a Media field to the "Default comments" comment type. Then I created an Article node, and from there could see what the Media widget and dialog looks like from a node page that can be commented on, which is themed in Bartik.
Here are the before and after screenshots.
Comment #56
phenaproximaTwo options come to mind about the above. Both are based upon the assumption that Bartik is closed, internal, and can be changed at any time.
Comment #57
lauriiiThanks for generating the screenshots @effulgentsia! We could work around the problem by copying the Seven styles to Bartik but I don't think it's the right solution. The designs are Seven specific, and since the styles are located in Bartik, they wouldn't be modifiable by other admin themes. For example, Claro would run into this issue pretty soon in #3062751: Media and media library. Also, this would only solve the problem in the scope of Bartik, and every other frontend theme would run into the same problem.
I think the right solution would be to solve #2195695: Admin UIs on the front-end are difficult to theme but that will likely take some time.
Comment #58
webchickThis one is particularly un-good. It looks like after this patch (or the other patch; I get all the patches confused), all themes are starting from the standpoint of “two copies of your thumbnails, and also not in a grid despite the button says ‘grid’” which seems... not... awesome... we might be taking "lack of theme opinionatedness" a bit too far here. :\
Comment #59
phenaproximaSpoiler alert: I applied the patch in #52, then copied all of the Seven stuff into Bartik. It looked great; screenshots attached.
So, if you ask me, copying all the Seven stuff into Bartik is a fine workaround. It's not the "right" solution, as @lauriii says, but it might be the best we can do in the short term.
Another option is for us to keep media_library.theme.css in the Media Library module, but go over it with a fine-tooth comb and only keep styles which keep the library looking reasonably good in most themes by default. But that sort of undermines the point of #3087456: Move some representational classes in Media Library to Classy and others to Seven and/or Claro, as appropriate.
Comment #60
lauriiiI discussed with @effulgentsia and @phenaproxima about #55 and we agreed that copying Seven styles to Bartik and Umami would be just a workaround solving this problem in our own product. This would still leave all other themes suffering from this. Also moving the styles to Classy would not be good because we know we want to modify these styles in the future. We decided that as a step to try to improve the UX on Umami and Bartik, we will add some basic layout rules to Classy to make the layout look less broken: #3089743: [PP-1] Media library layout is broken on Bartik and Umami. This should be done prior to Stable because we shouldn't be adding this type of rules after Media Library has been marked stable.
Comment #61
phenaproximaAt @effulgentsia's request, here is a complete patch incorporating the following:
Comment #62
phenaproximaRerolled following the revert of #3085908: Media library thumbnails are blurry/skewed in IE11.
In addition to a tab-corrected #52, this includes #3087456-118: Move some representational classes in Media Library to Classy and others to Seven and/or Claro, as appropriate and #3089743-12: [PP-1] Media library layout is broken on Bartik and Umami.
Comment #65
effulgentsia CreditAttribution: effulgentsia at Acquia commentedHere's some screenshots of the dialog right after you upload an image and before you've filled in the alt text.
Notice how in Bartik and Umami, the image is shown twice, and the Remove button is shown twice.
Comment #66
phenaproximaYup, that's a pre-existing condition, and covered by #2987921: Media Library add form should suppress extraneous components of image fields using a form alter, not CSS (as commented in the CSS):
This is done because, when uploading new image media, we need to hide all parts of the source field (which is an image field), except for the alternative text. We currently do that with CSS, and have for a long time.
Because we intend to remove it later and the upload is still usable, albeit ugly, I don't think this is something we should be fixing in Classy. The compromise I'd suggest, if anything, is to fix this temporarily in Bartik (just copy that entire CSS rule into it).
Another option would be for Media Library to implement hook_field_widget_WIDGET_TYPE_form_alter() and suppress those parts. But again, I don't think that should block stable, and it should be done in #2987921: Media Library add form should suppress extraneous components of image fields using a form alter, not CSS.
Comment #68
webchickWith strong gnashing of teeth, I'm gonna say that the screenshots in #65 are acceptable to ship with. There's a known bug, with a patch, and once it's fixed, all of the themes affected will go back to normal. (As opposed to the grid styling, which required all themes to do a thing to "fix" it.)
So count this as re-PM sign-off of this awesome initiative!
Comment #69
xjmI asked @phenaproxima if we can fold #3087126: Media Library "select all" checkbox does not have proper margin in RTL styling into this issue so that RTL sites look OK too. It sounds like an easy/small thing to add. (Hopefully.)
Comment #70
phenaproximaOkay. Added some workarounds to MediaLibraryTest to stabilize testAdministrationPage(), which is known to be flaky on local machines. Also rolled in a fix for #3087126: Media Library "select all" checkbox does not have proper margin in RTL styling, which I tested manually, and will close that issue.
Comment #71
phenaproximaAdded a release note. I know it's simplistic, but I'm not sure what else to say and couldn't find any other examples.
Comment #72
phenaproximaExpanded the release note a bit, based on what Layout Builder had in 8.7.0.
Comment #74
lauriiiThis CSS file is now empty. Should we just remove this and the library?
Comment #75
effulgentsia CreditAttribution: effulgentsia at Acquia commentedLooks like we generally don't use an
@api
annotation anywhere in core/modules or core/lib, despite https://www.drupal.org/core/d8-bc-policy still mentioning it. I only see one usage of it in all of 8.8.x: and that's inConfigEntityUpdater::update()
, which maybe is a remnant of an earlier time when we used it more. I'd suggest removing it from this patch, and finding or opening an issue to discuss whether we want to start using this annotation.Do we still need this?
Let's move these to the
media_library
subdirectory.Comment #76
phenaproximaThis won't fix the test failures, but...
Addressing #74:
libraries-extend
stuff in Seven and Classy.Addressing #75:
Comment #77
phenaproxima@lauriii pointed out that we were still attaching the
seven/media_library
library inviews-view-unformatted--media-library.html.twig
. Now that we're using libraries-extend, there's no need for that, so I removed it. I also had Seven overrideclassy/media_library
so as to ensure that the layout styling in Classy doesn't conflict with the styling in Seven, which totally supersedes it.Comment #78
lauriiiI don't have any more concerns. Good job on addressing all the feedback 👏
Comment #81
phenaproximaI suspect this will fix the tests. I don't know why MediaLibraryTest::testAdministrationPage() is having such trouble with, y'know, pressing a button to submit a form (and, weirdly, only in that one test...and even more weirdly than that, only failing consistently on testbot). But, this workaround seemed to help when I tried it in a burner issue.
MediaLibraryTest is such a terrible, horrible, no-good beast anyway that it doesn't seem so awful to put workarounds in it for now in the name of marking this module stable. 🤷♂️
Comment #82
seanBAll tests are green and as far as I can see all concerns are addressed and the work in all child issues has been merged in the main patch. Marking this RTBC. Great work everyone, thank you! ❤
Comment #83
phenaproximaExpanding the release notes with info about disruptive changes.
Comment #84
xjmI sat with @lauriii and we went through the many arcane changes from the several theme blockers. The pretty spreadsheet was also a big help!
There are a few out-of-scope cleanups to small things like inline comments, but they're trivial enough that I'm not concerned about it. The only things I'm still hesitant about are in the new
MediaLibrarySevenTest
:There's no documentation anywhere on this test class... At a minimum there needs to be a docblock here.
This will run all the methods of this huge, very unstable, oft-failing test again... are we doing that on purpose? And... do we have to? I still get emails literally every night from at least one of the twenty-odd test environments failing on this test.
It's good that it has test coverage, don't get me wrong. I'm just worried about this failing twice as much. In some ways it can be considered a "new" random fail too... even though it should also be fixed by #3066447: [random test failure] Random failures building media library form after uploading image (WidgetUploadTest) etc., it makes it twice as bad until then.
Both @lauriii and I initially thought this was a copypasta mistake. I guess these are indeed in
MediaLibraryTest
and therefore both the@inheritdoc
and the subclassing are intentional. But... erk.Why?
Should this have had its own issue?
Out of scope?
New assertion?
NW for the first point; others are open questions.
Sooooo close!
Comment #85
phenaproximaComment #86
phenaproximaI discussed the fate of MediaLibrarySevenTest with @xjm. On the one hand, she didn't want to remove valuable added test coverage. On the other hand, she didn't want to bump up the number of random failures. I can certainly appreciate that conundrum!
We agreed to rip MediaLibrarySevenTest out of this patch completely, and open a follow-up, postponed on the refactor of MediaLibraryTest, to restore it later. Since the planned refactoring of MediaLibraryTest will split it into several smaller, more stable tests, MediaLibrarySevenTest will eventually just be a short, well-scoped test that runs through the parts of the media library which are actually touched by Seven.
I will work to deliver a green-on-all-backends patch within the next few hours. Given the time constraint, @xjm authorized me to set that directly back to RTBC when it's ready.
Comment #87
phenaproximaPatch. No interdiff because this is identical to #81 apart from the disappearance of MediaLibrarySevenTest.
Comment #88
phenaproximaTagging for the follow-up discussed in #86.
Comment #89
phenaproximaOpened #3090983: [PP-2] Test Seven's changes to Media Library to restore MediaLibrarySevenTest as a proper, tightly scoped, well-written test covering Seven's modifications to Media Library.
And, as per discussion with @xjm, restoring RTBC on the assumption that all backends will be green.
Comment #90
xjmGoing on through my checklist...
So there are a few testing followups that are things we really should do for Media Library test, because it is very. Very very. Disruptive at the moment. These don't need to block marking the module stable, because with @phenaproxima's update in #87, it's exactly as disruptive before or after stable.
That said, I'm adding the issues to the checklist in the summary. If it's possible to fix them before 8.8.0, that would be really great. I'd also consider them blockers for adding the module to Standard so I'll update that roadmap too (if someone didn't already).
Comment #91
xjmMy docs feedback from #40 has been addressed so checking off docs!
Comment #92
xjmCKEditor integration has been addressed in the numerous must-haves in #2801307: [META] Support WYSIWYG embedding of media entities, and Content Moderation addressed in #2969678: Integrate Media Library with Content Moderation.
Comment #93
xjmComment #94
xjmChecking off more items that are already addressed... which is all of them!
Comment #95
xjmNooooOOOOoooo it doesn't apply... 😿
Reroll please? 😺
Comment #96
oknateHere's a reroll. The only difference was in the media_library.libraries.yml. The interdiff is a little weird, since it's a diff between the two patches.
Comment #97
phenaproximaAh, I see, the reroll was due to jQuery UI deprecations that were likely committed during DrupalCon Amsterdam. (We were depending on Sortable.) RTBC on the assumption that tests will pass.
Comment #98
JeroenTThe current patch does not apply for Drupal 9 since the following issue was committed: #3072702: Core extensions should not need to specify the new core_version_requirement in *.info.yml files so that 9.0.x can be installed.
I created a D9 version of this patch.
Comment #99
phenaproximaVerified that the changes needed to make #96 apply to 9.0.x are present in JeroenT's patch. Thanks!
Comment #100
xjmWe can't use PHP 7.1 test envs. for 9.0.x; I'll queue the 7.3 ones instead.
Comment #101
xjmSo this is where the patch failed to apply to 9.0.x, on the context lines. In 8.8.x it looks like:
Comment #102
xjmAdding credits for reviewers on this issue for people who have already commented.
Comment #109
xjmMoe credits for contributors for everything from the initial prototype to the roadmap itself...
Comment #117
xjmMore credits...
Comment #119
xjmComment #120
xjmThe ES6 transpilation for 9.0.x seems to be out of sync:
The 8.9.x/8.8.x version of the patch passes this check.
Comment #121
phenaproximaRan
yarn build:js
against #96 on 9.0.x.Comment #125
xjmCOMMITTED TO 9.0.x, 8.9.x, AND 8.8.x! WOOOOOOOO!
Excessive and profuse thanks to everyone who pulled together and made this happen in this release despite the many obstacles that came up for us. This is a huge milestone for the Media Initiative. Extra thanks to @phenaproxima for long-suffering. :)
Next step: STANDARD.... https://www.youtube.com/watch?v=-bzWSJG93P8
Comment #126
xjmDEFINITELY a highlight of this release. Also the release note about the theme shuffles should go in the release notes.
Comment #127
pookmish CreditAttribution: pookmish commentedI believe this is the issue an error has come from:
User error: "0" is an invalid render array key in Drupal\Core\Render\Element::children()
I found that in the function
seven_form_media_library_add_form_upload_alter()
there is an incorrect attributes key.$form['attributes']['class'][] = 'media-library-add-form--upload';
should be
$form['#attributes']['class'][] = 'media-library-add-form--upload';
My database logs are full of this error now.
Comment #128
oknate@pookmish. Nice find. Let's move this to a follow-up issue.
I added one: #3094397: Attributes key missing hash or pound sign in seven