- Issue created by @scott_euser
- 🇫🇷France prudloff Lille
See this comment 📌 All footnotes available as block for Layout Builder / Paragraph / ... Needs review .
This module implements a JS solution because the PHP solution discussed in the patch does not work in some cases. - 🇬🇧United Kingdom scott_euser
Thanks for the quick reply!
Sometimes we trigger a render of a field but we don't display it on the page (for example, because we want to use it in a meta tag), so we don't want its footnotes to be added to the block.
If you check the 'disable' option it won't render it, but you can still use the `FootnotesGroup` to do some other action.
Sometimes fields can be rendered after the block (with lazy builders, for example) so the block will not have their footnotes.
Yes I can see lazy-built content would not show up there with the current approach, would have to do some sort of pre-render.
In general, I see this module does a lot of repetition of old 3x code well beyond just the JS issue. Perhaps it can be cut back significantly now and focus just on the JS rendering alternative to the Footnotes Group Block? Ie, isn't all you really need a separate block + this JS file? Perhaps we can:
- Add an option to the Footnotes Group block configuration to opt for JS solution
- When ticked load your footnotes.js file/library
That way others using Footnotes can more easily benefit from your approach as well.
I completely get why you copied it over of course, the module did not get much attention for some years and a series of patches were needed to use the module in many cases. I only recently reached out to the original maintainers to get co-maintainership to take it on.
- 🇫🇷France prudloff Lille
In general, I see this module does a lot of repetition of old 3x code well beyond just the JS issue. Perhaps it can be cut back significantly now and focus just on the JS rendering alternative to the Footnotes Group Block?
What do you mean? The module only contains a block that renders an empty placeholder and the JS file.
- 🇬🇧United Kingdom scott_euser
Ah sorry (facepalm moment). I was looking at the git source of the 3x branch of Footnotes. Too many tabs open :)
Anyways, do you want to consider merging it in via an configuration option in the Footnotes Group block? - 🇫🇷France prudloff Lille
I would not be against that but I don't have time to contribute a patch right now.
- 🇬🇧United Kingdom scott_euser
Okay if I start an issue then myself and copy over the code? I can add a test for it + mark it for review. Then ideally if you review it and check you're happy with it, that would be great.
Thanks!
- 🇬🇧United Kingdom scott_euser
Created ✨ Provide a JS alternative to the PHP-driven Footnotes Group like Footnotes All Block module Needs review to track progress on this.
- 🇬🇧United Kingdom scott_euser
Okay this is added now to ✨ Provide a JS alternative to the PHP-driven Footnotes Group like Footnotes All Block module Needs review along with test coverage. I roughly took your code, added some more checks + support for the block itself being loaded via lazy builder + removed dependency on jQuery. I added credit to you to that issue there as well. When you have a chance, would be great if you can review and see if you are happy with it.
Thanks!
- 🇫🇷France prudloff Lille
I tried the patch and I confirm it works correctly and does the same thing as footnotes_all_block.
Do you think we should provide an upgrade path that automatically converts the footnotes_all_block block to the new block?
- 🇬🇧United Kingdom scott_euser
Good idea, should be fairly easy I think. Let me give it a quick whirl
- Merge request !2Provide update hook to convert to footnotes module and uninstall → (Merged) created by scott_euser
- Open on Drupal.org →Core: 9.5.x + Environment: PHP 7.3 & MySQL 5.6last update
10 months ago Waiting for branch to pass - Status changed to Needs review
10 months ago 3:24pm 15 January 2024 - 🇬🇧United Kingdom scott_euser
Added the composer.json change so it forces people to decide to upgrade to 4x first for footnotes. If they stick to 3x until it has a stable release (as they should) it shouldn't upgrade I believe, as composer should flag the newer version as incompatible and not update this module.
- Open on Drupal.org →Core: 9.5.x + Environment: PHP 7.3 & MySQL 5.6last update
10 months ago Waiting for branch to pass - 🇬🇧United Kingdom scott_euser
When you have had a chance to check this, if you could also comment on ✨ Provide a JS alternative to the PHP-driven Footnotes Group like Footnotes All Block module Needs review please and I'll get that merged into `footnotes` as well. Thanks!
-
prudloff →
committed 18b6a144 on 1.x authored by
scott_euser →
Issue #3414345: All footnotes grouped in a block now available in...
-
prudloff →
committed 18b6a144 on 1.x authored by
scott_euser →
- Status changed to Fixed
4 months ago 5:04pm 15 July 2024 Automatically closed - issue fixed for 2 weeks with no activity.