I recreated the change on 11.x, but since I can't edit MR !10210 (and I'm tired), I created a new one. Can someone close MR !10210 for me?
ambient.impact → created an issue.
ambient.impact → created an issue.
ambient.impact → created an issue.
My original implementation of dark mode toggling was primarily CSS using @media (prefers-color-scheme: dark) {}
, which not only would have avoided having to store anything by default, but also removed the additional problems with having to check storage with JavaScript, i.e. no chance of brief flashes of light mode unless you used an inline script which is a problem with CSP, etc. See
#3161904-27: Auto Darkmode option →
for my concerns at the time.
That's what I get for having the tab open with the form state saved. 🤦
Right you are. Thanks!
Also fixed some typos in my summary.
An alternative if you don't want to install Ultimate Cron just to disable Warmer cron is the Hux module → 's ability to replace a hook implementation like we did on Omnipedia.
I can confirm that I'm also seeing this problem where a class that only uses ReplaceOriginalHook
didn't get discovered no matter how many container rebuilds or full cache clears I tried, and I thought I was losing my mind until it dawned on me to search the issue queue. I can also confirm that adding a #[Hook('...')]
to the class does seem to cause it to be discovered.
There's a module for that: https://www.drupal.org/project/image_style_warmer →
Added screenshots.
ambient.impact → created an issue.
Added videos.
Added screenshots.
ambient.impact → created an issue.
Ah, I see I'm not the only one that was utterly confused by the cryptic "LoadJS" error. It turns out the library did this to keep the size down, which I personally feel was a bad move because it's so confusing and non-helpful. I opened a pull request to change it, but since they still want to keep the size down, they're only willing to throw the bundle ID as the error message rather than a fully descriptive thing that says why it was thrown. Glad to see this is being handled on the Drupal side.
Not to dunk on the author of that even more, but I'm in agreement with @fathershawn, and would also highlight:
An attractive future direction might be to re-implement Htmx in React:
- The server sends a JSON blob that React converts into virtual DOM
components.
...which baffles me because it sounds like this person doesn't understand the core philosophy behind htmx and similar approaches. Sending a JSON blob for React to convert into a virtual DOM has nothing to do with htmx, and it's in fact entirely antithetical to htmx as I understand it.
Even after applying the patch from the merge request and clearing caches, I'm still getting the fatal error. Is there a step I'm forgetting to get this to work?
Added Turbo to HTMX guide link.
It's worth taking a quick look at Turbo, if only to get a sense of what we would have to recreate with HTMX. I did a bit of poking around the HTMX docs just now and realized they have a guide for migrating from Hotwire Turbo, so that'll be handy.
@fathershawn Most of that I'm aware of. I think the biggest challenge in implementing the current functionality in HTMX is that we'll have to re-implement a lot of stuff that Hotwire Turbo does out of the box, but building on your module will help get there a bit faster when I (or someone reading this?) have the time and energy for it.
My impression is that HTMX seems to more of a low level toolkit that you build with - would you say that's accurate?
ambient.impact → created an issue.
If someone is willing to implement it in a robust and reliable way, I'd be happy to review and merge it. I can see the value of it, but my time and energy are limited right now so it's not a priority for me.
ambient.impact → created an issue.
I took a quick look but Pinto seems to require more set up than I would like. I feel that it's missing what I love about Hux: I can just install Hux, drop one or more classes into src/Hooks
with concise PHP attributes, and it automagically works. I don't know enough about the inner workings of Drupal's theme system, so maybe that's just not currently possible, but I feel like Hux has the winning formula from a DX perspective.
Doing a bit of research on this and came across the Preprocess module → , which seems pretty straightforward to use until core or someone else comes up with a better solution. It's not quite as elegant as Hux, but it's not terrible either.
Moved 📌 Turbo: Drastically simplify installation for end users Active to postponed as immediate goal is achieved.
We still need a proper build process as detailed in the issue summary but the commits above are good enough for now and accomplish the immediate goal.
Moved 🌱 Turbo: Tracking issue of core and contrib behaviours that need fixes to correctly detach and then attach Active to ongoing from blocking.
Marked the core/misc/announce.js
item as complete.
Added 📌 Turbo: Drastically simplify installation for end users Active to blocking issues.
ambient.impact → created an issue.
Removed 📌 Turbo: Configuration and form for toggling Turbo features Active as it's not really relevant to a minimum viable implementation right now, though might be done down the road.
ambient.impact → created an issue.
Added 🐛 Turbo: Drupal messages are lost if Turbo does a full reload Active .
Removed 🐛 Turbo: Remove behaviours from Drupal.behaviors when Turbo removes their Active from issue summary.
Added postponed issues and ongoing issues lists, and moved linked issues where appropriate.
Removed some lingering old stuff that's no longer relevant.
Significant restructure of issue summary; moved the old stuff to a section at the end as archived notes.
Going to close this as won't fix because there's no realistic way we could do this without breaking various things.
Moved 📌 Turbo: Automated tests Active to non-blocking.
Added item for Drupal\refreshless_turbo\Value\RefreshlessRequest
.
Added list item for page cache request policy.
Removed session item (not actually true) and added page cache request policy item.
Added References section with links to the HTMX module.
Added a few remaining tasks from my comments.
Moved 🐛 Turbo: JS aggregation causes the RefreshLess JavaScript to evaluate more and more times on navigation Active and 🐛 Turbo: Drupal messages are lost if Turbo does a full reload Active to blocking issues.
Link to 📌 Turbo: Automated tests Active for test details.
Added CSS and JS aggregation list items.
Changed ⌛️ emoji to ⏳ as the former is a finished hourglass while the latter is what we want: an in-progress hourglass.
Reordered "Remaining tasks" and "Follow up tasks", and moved some stuff between them where they make more sense.
Updated status of a few items.
ambient.impact → created an issue.
Whoops, it looks like I accidentally replaced the issue summary with my comment. One of the maintainers should revert that if possible. 😬
Minor grammar fix.
Added remaining tasks and follow up tasks.
Heavy rewrite of issue summary reflecting current understanding of the problem and solution.
Added 🐛 Turbo: Drupal messages are lost if Turbo does a full reload Active to non-blocking tasks.
Updating title as we now know what needed to be done and why.
ambient.impact → created an issue.
In my case, I have a couple of sites with complex content that makes heavy use of the caching API (cache tags and cache contexts) and we try to avoid clearing the cache unless absolutely necessary. We even have a custom drush.yml
with this:
command:
updatedb:
options:
# Don't clear the cache by default to avoid unnecessary work.
#
# @see https://www.bounteous.com/insights/2020/03/12/automated-drupal-deployment-and-rollback-recipes/#don%E2%80%99t-clear-all-caches-instead-selectively-clear-what-you-need
cache-clear: 0
Which I suspect may be part of the reason we also got hit with this Drupal\config_split\Plugin\ConfigFilter\SplitFilter
error, despite following the 2.0.0 instructions and had drupal/config_filter
required explicitly. Thankfully, this happened on a staging site so I could just restore a database back up and try a few things. What ended up working in our case to avoid a full cache clear was:
- Upgrade Config Split in a dev environment; update database; export config; commit/push.
- On the target site (staging/production) do
drush pm:uninstall config_split
- Deploy the updated codebase and config files.
drush -y updb
and thendrush -y config:import
(our deployment process does this automatically)- Config Split 2.x is now installed and using the 2.x config without a problem.
The workaround I've committed is okay for now, but still results in brief flashes of incorrect specificity, so I'm going to set this as postponed for the time being.
Linking screenshots in issue summary.
Added screenshots demonstrating the issue.
Alright, so I think I have it mostly working. It should now be aggregating and
attaching only what's not already on the page, with the following caveats/problems yet to be resolved:
- Our module JavaScript is set to
preprocess: false
for now because it doesn't seem possible to group ourselves into a separate aggregate; see ✨ [PP-1] Allow setting aggregation groups for js files in library definitions Postponed . Ideally, we would be grouped into one aggregate that contains just therefreshless_turbo/refreshless
andrefreshless_turbo/turbo
libraries so that we could take advantage of core's minification and gzipping. - We have to special case the
refreshless_turbo/refreshless
andrefreshless_turbo/turbo
libraries to ensure Turbo is always attached even with the additive libraries logic, but never attached or aggregated more than once because bad things happen when Turbo loads and evaluates itself a second time. - There's some duplicate logic between the middleware and our decorated asset resolver that I'm not happy with but I couldn't get it work well enough without it. I'll probably rewrite some of it into an attachments processor (see the HTMX module's
HtmxResponseAttachmentsProcessor
) to hopefully remove the duplication. - This re-introduces the same CSS specificity symptoms as seen in
🐛
Turbo: CSS files merged into the on Turbo navigation always appended instead of preserving full page load order, causing CSS specificity issues
Active
but now when aggregation is enabled as well: the cause now seems to be that the base theme's (Claro) CSS for dropbuttons and vertical tabs is additively aggregated (since they're defined in separate libraries and not on every page) while Gin compiles dropbuttons into its big
gin.css
that gets attached to every page and uses a fixed weight value on itstabs.css
library definition rather than listing Claro's as a dependency. The best course of action may be to rework the additive aggregation so that it only applies to JavaScript and not libraries as a whole, leaving CSS to aggregate as it did before, non-additively; this may be doable via our decorated asset resolver in thegetCssAssets()
method by checking if the current request is a Turbo request and then temporarily removing the already loaded libraries, calling$this->decorated->getCssAssets()
, then restoring the already loaded libraries before returning the CSS assets. - It's unclear if this needs a cache context because this seems to be working with caching, but my gut is saying that we should have a cache context and set up a clear distinction between a full page load and a Turbo/additive load, because there's a bunch of stuff in the code that just YOLOs it without a care which might backfire in the future.
So it still needs work but some progress has been made. The hardest part of this is working without an easy way to visualize what libraries are being (additively) aggregated and attached to a page. Setting up some tests for that would help, but I would also want to implement console logging of what new libraries get added, much like the 8.x-1.x branch has, and may something a bit more visual would help as well.
Added section discussing the HTMX contrib module → .
Merged and setting back to active. Thank you, bot.
Right you are. The param hints have been reverted.
Also ignore the 1.0.x branch in the fork. Accidentally pushed to that because I somehow had that set as the upstream instead of my own. Had a senior moment.
ambient.impact → changed the visibility of the branch 1.0.x to hidden.
I merged in the latest 8.x-1.x changes, i.e. up to 8.x-1.4-rc1 ;)
ambient.impact → made their first commit to this issue’s fork.
ambient.impact → created an issue.
Slight grammar fix.
Rerolled the patch because the previous couple of patches broke Drupal\Tests\media\Functional\testResourceDataAlter()
; notable changes:
- Fixed this syntax error that snuck in; see if you can spot it ;) :
$resource_url = $this->container->get('media.oembed.resource_fetcher')+ ->fetchResource('https://publish.twitter.com/oembed?url=https://twitter.com/Dries/status/999985431595880448');
- Added
void
return type totestResourceDataAlter()
for consistency with other core changes to tests.
So I tried to implement a solution based on @catch's idea and it sort of works but not quite. It doesn't yet have a cache context to vary by and it breaks Turbo since the Turbo JavaScript isn't found on the page so Turbo does a full page load, and I feel like piggybacking on ajaxPageState
might not be worth the potential headaches down the road if that changes.
To do list/ideas
- Implement a cache context that varies based on a hash or compressed URL query string because we'll likely need to add that to the page
#cache
. - Port over some of the 1.x
refreshless_page_state
logic and don't spoof or useajaxPageState
. - Also don't forget
RefreshlessRenderer::validatePreconditions()
for security from the 1.x branch. - Turbo and RefreshLess stuff needs to be handled separately and excluded from being removed on subsequent pages because doing so causes Turbo to relinquish control and let a full page load occur. We may have to decorate another one of the core asset services to group the RefreshLess and Turbo stuff into its own aggregate since setting it all to
preprocess: false
isn't great. - Implement the PHP logic somewhere to exclude the already loaded libraries somewhere; our decorated
AssetResolver
would make the most sense since it's central to the whole process.
Core stuff to use as reference
Ambient.Impact → created an issue.
Fixed a silly typo and added missing link to \Drupal\Core\Form\ConfigTarget
.
Ambient.Impact → created an issue.