🇳🇿New Zealand @atowl

atowl → created an issue.

I tested again with the navigation module enabled, and the problem doesn't exist, but the secondary toolbar gets a bit messy up the top

So i guess yes? you should make it a dependancy, I personally dislike (strongly) however not being able to set the menu to go horizontal, i've change the MR to check if the core_navigation variable is set in the template, to then include the file. that way it works either way.

Whats the direction that gin and this module are taking? it appears they are locking us into a vertical only menu?

atowl → created an issue.

atowl → created an issue.

Think the current screen shots are for an older version of drupal, and prehaps outdated.
I've uploaded some new ones, showing what its currently like in D11, and have left the original screen shot showing what i think the OP wanted it to look like if the extension is loaded.

The summary of the image should still be shown if the extension isn't loaded.

@smustgraves, little harder to do this as an automated test as we need to mock the extension checker so we can return false in check.
However, we could refactor it locally and then mock the call to the local function?

Also, i'm wondering if the proposed solution doesn't quite match the screen shots that were provided now? The proposed solution is to just grey out the options if the extension isn't loaded, but the screenshots simply remove the options. Is that more what we are after? with maybe a message about why?

kim.pepper → credited atowl → .

atowl → made their first commit to this issue’s fork.

atowl → made their first commit to this issue’s fork.

So i think with the hooks that have been been implemented in #3504396, this issue can now be closed.
I have created a module that will do the override, which seems to work just fine.

Thanks

Hi Shelane,

Yes its on a per node basis.

i have thought about changing this to a sub-module of our own, so having it installed would just implement the bitly link override.

Hi @isholgueras,

We're upgrading from 4.0.19 -> 4.0.20.

Appending to the reproduction, we noticed it during a drush deploy.

I'm not a fan of changing the getVersionIdentifier parameter to ?string, because it should never be null i don't believe.

atowl → created an issue.

atowl → created an issue.

Hi there,
Thanks for reporting this.
I am trying to reproduce this locally, is there anything you have missed out?
I have a
* vanilla Drupal 10
* Google Analytics module installed
* Eu cookie compliance.
* I am using a valid google analytics ID
* copy and pasted
https://www.googletagmanager.com/gtag/js
modules/contrib/google_analytics/js/google_analytics.js
into the EU Cookie compliance disabled scripts box.
But i do not see the error on the main page?

Thanks.

Not that i'm an expert in caching, but we might have to put some of the caching context back.
Keep the Language, and theme out, since it's done in the services file already, and leave in the domain module check? This will leave specific domain caching intact?

Thoughts?

Since this is labelled 7.x, should it be closed?

I think what you've proposed sounds resonable, rather than attempting to remove or disable the EUCC module we should probably just disable the banner (with a message saying please configure Klaro) and enable a Klaro banner instead. I feel its like almost like disabling our module but not quite.

To address your scenario, which is a great example. I would say that if a site migrates to Klaro then the consent banner should be shown again. My reason is that the site has changed consent managers so this in my opinion is almost like changing privacy versions.

I have spent a bit of time to do feature map that EUCC has and where they map to in Klaro

I'm ignoring any style demands presently.

I'll attach the spreedsheet for now, i could also do it in a google sheet instead if its easier?

Sven, could we just provide a migration a button from the /admin/config/system/eu-cookie-compliance area?

I think i agree with not digging into reading the theme css, but we could provide a style sheet with any text/background colors, heights/widths etc, that they have defined if the switch for include minimal css is off.

atowl → created an issue.

Hello,
I wondered where we are with this issue? As I've tried both the MR patch and the patch in #50.

As i see it the MR works, if you don't have revisioning turned on. It gets triggered once a block is removed, but because there would be a revision of the page, it returns early, not removing the block, and then is not triggered again, to actually clean up any block that was placed, (once revisions have been deleted).

For the patch in #50, this works.. kinda. Because it makes a new primary key it is truncating the inline_block_usage table. While this doesn't destroy a page layout, you can re-populate this table by saving the page, writing back the usage to the table. BUT if you run cron inbetween that time, it sees that the layout_entity_type and layout_entity_id fields are null, and will start removing the blocks, which then DOES destroy page layouts, like all of them. Not ideal.

I am wondering if we should start back with the first MR, but hook it into the page save.
For psuedo code It would
* Iterate over the blocks associated to that node
* Skip if included in any revisions
* Else set layout_entity_type, layout_entity_id to null (layout_builder_cron() cleans this up later)
* Not reusable block? delete from block_content table also.
I am not sure however if there are more places to remove the block from if any.

Now we also would need to do a cron task, (or just on a dbupdate), to remove unused blocks that are are already in the block_content table.
i think this is as easy as
* Get id from block_content table
* If id is not in inline_block_usage table, delete.

I get the feeling that there should be some bit here that looks into the blob of the node__layout_builder__layout table (or another table) for usage. But unsure which, if any.

thanks

atowl → made their first commit to this issue’s fork.

Hi guys,
It's been a while, and looking back at these patches they weren't so great.
Instead, i've created a new fork with all the patches rolled in.

Changing status to reviewed and tested,
Maintainer can choose which fork they want to include.

Hi @ahsannazir,

Yes, looks like that works as well. i think using the variable(s) --input-padding-vertical is better.
So the question now is really, which is more correct? to place the padding on the class itself or on the element within the class.
Happy either way, I'll let the maintainer decide.

Updated the issue summary with reproduction steps.
Very similar to my other issue, requires the the chosen module to be installed.

Have updated the reproduction steps.

atowl → created an issue.

atowl → created an issue.

Is this still an issue? I applied your patch and didn't see any changes.
It may have already been fixed via https://git.drupalcode.org/project/gin/-/commit/e7f72df9c693dbcdbadeabdb...

atowl → created an issue.

Found an issue with this approach, the max-age should not be set to 0, then drupal never caches it.
instead, so we can use caching, set 'contexts' to 'url'.
Not sure if its the right thing to do, as i don't know enough about caching here.

Re-rolled patch for version 4.0.7

Re-rolling patch updating for version 4.x

AlMunnings → credited atowl → .

Sorry - last patch didn't include the shift of the configuration out of the if statement. This caused a get on null error.
Fixed.

We ran into some caching issues with this, where it would not get the new entity (ie page) due to being cached already.

To solve this, moved the logic for doing the preferred link into the preprocess hook, and into its own render array with `#cache => max-age = 0`
As i understand it, it will now always re-render that array into the template. And hence always get the preferred link.
I haven't done anything quite like this before so if that's the wrong approach let me know.

Thanks

Hi,
So upgraded field_group, and attempted to make a group again. Still didn't seem to work.
Exported the configuration, it looks like

uuid: e33aede0-580e-4073-9414-23b6df01a6d3
langcode: en
status: true
dependencies:
  module:
    - field_group
    - link_attributes
    - menu_link_content
    - menu_item_fields
third_party_settings:
  field_group:
    group_advanced:
      children: {  }
      parent_name: ''
      weight: 20
      format_type: tab
      region: hidden
      format_settings:
        id: ''
        classes: ''
        description: ''
        formatter: closed
        required_fields: 1
      label: Advanced
    group_feature_box:
      children: {  }
      parent_name: ''
      weight: 20
      format_type: tab
      region: hidden
      format_settings:
        show_empty_fields: 0
        id: ''
        classes: ''
        description: 'Contains all our feature box requirements'
        formatter: closed
        required_fields: 1
      label: 'Feature Box'
id: menu_link_content..default
targetEntityType: menu_link_content
bundle: ''
mode: default
content:
  langcode:
    type: language_select
    weight: 2
    region: content
    settings:
      include_locked: true
    third_party_settings: {  }
  title:
    type: string_textfield
    weight: -5
    region: content
    settings:
      size: 60
      placeholder: ''
    third_party_settings: {  }
hidden: {  }

Does this help at all?

Hi There,

So looks like we're on version 3.1.0 of drupal/field_group, i haven't tried to export the config yet, but i'll try that.

Re-rolled patch for branch 4.x

atowl → created an issue.

Hi,

Is the test a failing test or is it part of the patch that is wrong? we'd like to use this patch.
Thanks