🇫🇷France @flocondetoile

flocondetoile → created an issue.

Looks good and fix the issue

Hello,

Just updated Gin.
Looks like the value of
$widget_type = $form['status']['widget']['#type'] ?? FALSE; is alsways false in my case.

Because the #type to check is in $form['status']['widget']['value']['#type'] and not in $form['status']['widget']['#type'].

And so the status checkbox doesn't go into the gin sticky actions / status container.

Am I alone ?

A field formatter is not in the scope of this module. It's not the right way to support this.

No thanks. I should manage it, I think.

There is a parameter in simplenews.settings.yml named "hash_expiration" which is hardcoded with 86400 value (so 24h).
This settings expires subscribe / unsubscribe links sent in a newsletter.

This parameter is not configurable in the settings UI.

I guess that make this setting configurable would allow to set the period after which the links are not valid anymore. But 24h (the default value) seems really short for newsletters.

A simple workaround is to change this settings in the config file, and then reimport this configuration (drush cim).

It's because you have request one time the -dev version of the module. So composer search always for the .git directory. Simply run composer remove drupal/toc_js and then composer require drupal/toc_js ^3.0 and you then will fetch the tarball version.

flocondetoile → created an issue.

flocondetoile → created an issue.

flocondetoile → created an issue.

Not sure this is the job of this module. Should be more relevant to improve form ids for view exposed form no ? And to have, as for node form for example, a base form id (the current one) and a specific form id for the given view. But this is a core issue.

Hello,

Could you be more explicit on the why "The module should update the composer dependencies to support html2text/html2text (^4.3.1)" ? Did you encounter conflict ? You just want the latest version of the lib ? Other reason ?

I just want to have more context.

flocondetoile → created an issue.

Looks good and works fine.

I guess you choose to apply a scroll-margin-top to each heading of the TOC, instead of a global scroll-padding-top to the html element (and so simpler), because it's let us more control to vary (if needed) this behavior per heading (via theme or other magic css) ?

Hello @netsilver,

The "only" D10 compatibility patch for this module doesn't really make sens, as simple_sitemap module is compatible D10 with its 4.x version. And 4.x version is a major rewrite (again) of the module. So this module needs too to have a major rewrite to be compatible with 4.x version of simple_sitemap.

I am a bit tired of theses successives major rewrites for each major version of simple_sitemap module. I don't want rewrite this module to be compatible with 4.x version, and then have to rewrite it again in a few years to be compatible with the next 5.x version. If you want do it I can give you maintener access.

If this module must be rewrite again, I want to do it without any dependencies on any others modules. In others words, the next major rewrite of this module (if it happens one day) would be a module which generates for each microsite a sitempap.xml in an independant way.

flocondetoile → created an issue.

Need to know which field generate this issue. Please provide steps to reproduce this error.

Nice. My TOCs are now fixed.

Yes thanks

as a dirty fix, this resolve the case

diff --git a/assets/js/tocjs.js b/assets/js/tocjs.js
index 443728a7cf..7c3af11e05 100644
--- a/assets/js/tocjs.js
+++ b/assets/js/tocjs.js
@@ -385,6 +385,15 @@
         lastLi = li;
         currentLevel = level;
       }
+      else {
+        const ul = document.createElement(opts.listType);
+        currentLevel = level;
+        ul.level = level;
+        ul.appendChild(li);
+        lastLi.appendChild(ul);
+        currentList = ul;
+        lastLi = li;
+      }
     });
   }

But defaulting to level 1 is good.

flocondetoile → created an issue.

Don't understand what is the issue.

Committed. Thanks.

Committed. Thanks

Committed. Thanks

Open to add a base field (type reference) on the menu_link_content entity and swicth the storage on this field.

Thanks. Committed

Thanks

Thanks. Committed.

Still having error "InvalidArgumentException: Class "\Drupal\file_delete\Form\FileDeleteSettingsForm" does not exist." with MR8. Investigating. I don't understand the Class form is well here and the namespace is correct and there is no typo...

Patchs 26 and above are wrong because the ymiss the FileDeleteSettingsForm Class.

InvalidArgumentException: Class "\Drupal\file_delete\Form\FileDeleteSettingsForm" does not exist.

Nevermind I think we should go with MR8 (or patch 25), and discuss about this third option about deleting referencing entities in another dedicated issue. Consequences are potentially hugh, it's not only deleting media referencing the file deleted, but also any other content entities which reference this file (node, term, etc.) in any of its field.

Did you run this patch / MR on a real D11 site ? I saw you RTBC your own commit.

For step 2 I am not sure the module have to support this. If you are able to create node programatically for a project, I guess you can handle yourself the value to set on the node given settings of node type.

Yes.

Only an alpha ? I was thinking at a stable version :-), or at least a beta release. What do you think ?

#3 yes we can

However, it does not exempt the heading when using the second version - i.e without the > and I as far as my understanding of selectors goes, the second case should also cover the first?

In fact no.

In fact, the selector :not(.is-a-group-title-Yes) h2 match all h2 inside the container. Because even if the direct parent of the h2 has the class "is-a-group-title-Yes", then the parant of this parent don't have this class, and so the h2 inside this parent is also targeted by the css selector :not(.is-a-group-title-Yes) h2 because there is always a parent (ancestor) of the h2 which dont have this class.

It's a css selector issue here

@mably ok with the decodeHtml.

@SirClickalot

However, it does not exempt the heading when using the second version - i.e without the > and I as far as my understanding of selectors goes, the second case should also cover the first?

Yes it should do. Could you check in your console, in debug mode, if the whitespace in the selector not(.is-a-group-title-Yes) h2 is not escaped too ?

And this depends a lot of the front theme used (sticky navigation bar, etc. etc.) I guess.

@mably feel free to close issues if necessary :-)

+1, yes

But need to be updated with the new TocJsPerNodeBlock too, added in 3.x

Yes

As we didn't have feedback for this feature request, I guess that customizing the css selector for the heading has done the job.
There is still the case that some heading have a style inline applied (and not a CSS class) which hide them, but I think this is a narrow case.

Let's wait a little to see if we have some more feedback, and if we need really a new setting like "check visibility headings" before adding these heading in a TOC.

If you think @mably it's a usefull feature, I let you chosse to add it or not.

So the feature request is resumed now to : "back to top" to a custom anchor (and not necessary the first heading). Close this as outdated as 3.0.x version allows to scroll to top of page now. Feel free to reopen if needed

Well in fact I wonder if an update hook is really necessary. In fact you rewrite the back_top_top feature to send user to the top of the page. And do what the feature tell / announce.

In most case, if the TOC is not sticky then the back to top link sent users to the top of the page because the TOC was above the content (A TOC below content is not relevant). And if the TOC is sticky, sending to the corresponding element in the TOC is in fact not really useful, where as sending to the top of the page can be useful.

So I suggest to let existing config as is. And this config which send user to the corresponding element in the TOC in 2x, will send users to the top of the page in 3.x. I think the behavior will be the same. Personnaly I never enabled the back to top feature with a sticky TOC.

You need to edit and change the machine name of your block instance when you create it.

Unistalling big pipe is not a solution, indeed. But we knows that the duplicate links are because of the multiple ajax request done on the page by Big Pipe.

No, back_to_top in 2.x was going back to the corresponding item in the ToC, as does back_to_toc in 3.x. And anyway the difference between these two behavior is not really important.

I guess the two (or X) back to top label are added because there is an ajax Refresh request done on the page. Or may be because of Big Pipe. If so try to disable Big Pipe to see if the rrot cause comes from here.

With Olivero theme, I don't reproduce it.

Which theme is used ? Which module (custom/contrib) with JS stuff is loaded on the page ?

OK so thie behavior of the back_to_top has been replaced with the behavior of back_to_toc, and now the back_to_top only send at the first heading.

Does it means that for users actually on 2.x versions, we need to write an update_hook for moving currents settings back_to_top and back_to_top_label in 2.x to back_to_toc and back_to_toc_label new settings ?

For green tests ^^

@mably just given you access on the project (write to VCS, edit project, maintain issues and releases)

@mably, before release a new stable version I wonder what are the difference between the existing settings back_to_top and back_to_top_label and the new settings you introduce back_to_toc and back_to_toc_label ?
Looks like they do the same thing but not very sure because lit seems that you kept all of them volontary ?

added mably as co-maintener

Comitting the mr11 on 3.0.x branch with theses slighty changes

- function toc_js_update_8001 not removed (we can't remove this hook_update even if empty because the number 8001 is registered on site which played it)
- toc_js is always dependant of core/once library. Re-added so the dependency

See interdiff

Could you try this patch ? If this don't resolve the bug (it's a blind patch), so I need context to be able to reproduce this issue.

On a fresh install on 10.3.2 I didn't succeed to reproduce this issue.

I test on a project which used intensively toc_js.
I have to change some css properties because the height of the sidebar was only the height of its containing elements, and not the height of the page content. Except this, yes this work fine, only if the toc_js element has also the css rule top: 0 ;(or any value for the top css rule) with the position: sticky;

Also you have still the sticky_offset property (which in theory set the css rule top: STICKY_OFFSET; for the sticky toc_js element) and I see you set this in the rewritten library embedded.

This simplifies the code enormously. Very cool.

I'll give a try on a real project.

Such improvment need at least a new branch. Just pushed a 3.0.x branch.

Thanks. Looks very promising.

>This version takes also profit of new CSS features available to modern browsers like the sticky positioning.

About the sticky positionning I saw you deleted the settings sticky_stop and sticky_stop_padding which allows us to set a selector for stopping to stick the TOC (For example I don't want the TOC to continue to be sticky when scrolling down in a big footer). Does it means that feature has been removed or is handle differently with modern browsers ? If this has been removed, this is a BC break for current users of this module, and i believe we should re-add it.

Anyway your work is awesome. And it would be nice to not have anymore a dependency on a third-party library !

> As I said already, we are willing to merge everything black to toc.js if you are interested.

To be interested, we need to know why you fork this project (excluded the need "to move fast"), and what you have added as new feature. Looks like you fork too the toc.js library itself and embed it directly in your code. Anyway the best way to discuss this is in a dedicated issue and not by hacking any issue/report done on this project.

@mably, it would be nice to stop spamming this issue queue with advertising for your fork. And also I don't understand why you fork this module instead of working (together) on the original, in a more collaborative way.

flocondetoile → created an issue.

New release published.

Fixed. Thanks for reporting.