🇺🇸United States @agentrickard

The CSS for this is not behaving as I expect, but it is nearly there.

It looks like we can use hook_menu_links_discovered_alter()

See ✨ Support the gin theme Active for an example.

agentrickard → created an issue.

agentrickard → created an issue.

I pushed a change to the branch that worked for me locally. I am going to commit the config change now while we test the update hook.

I can see this second problem on a fresh install. The update hook is not working.

Thanks for the reviews. This has been merged.

We might have to use dynamic routing to support both. See https://www.drupal.org/docs/drupal-apis/routing-system/providing-dynamic... → . With that approach, we could dynamically set the path based on whether Toolbar or Navigation were installed.

I still don't quite understand if Navigation will ever get past its "Experimental" status, especially given https://www.thedroptimes.com/49399/drupal-core-adopt-gin-admin-theme-rep...

I committed the original logo from dandolorion because I prefer the minimal aesthetic.

matthews → credited agentrickard → .

From the node_access subsystem perspective, this seems fine.

From a module developer perspective, it's also fine, as I consider node_access_view_all_nodes() to be an internal function and don't call it directly. Even if I did, there is an OO replacement I can use, and this code directs me to that.

Here is perhaps a better example of the issue.

The date is being stored in the database as an incomplete ISO Timestamp (e.g. "2025-01-14T22:30:00"). Storing in this way instead of as a Unix Timestamp -- which assumes GMT -- doesn't give a developer enough context to trust the value.

Consider:

$event_instance->date->value =>  "2025-01-14T22:30:00"

\Drupal::service('date.formatter')->format(strtotime($event_instance->date->value), 'custom', 'M j, Y g:i A') => "Jan 14, 2025 10:30 PM"

To work around that, we either have to add the TZ offset to the ISO string before conversion, or calculate it afterwards.

Here's what I ended up with (which should be daylight savings compatible):

      $event_instance = $item['content']['#eventinstance'];
      // Extract the event instance date and URL.
      // The date is stored in the 'date' property of the event instance, but
      // that is offset by the timezone, so we need to format it correctly.
      $timezone = \Drupal::config('system.date')->get('timezone.default');
      $date = date_create($event_instance->date->value, timezone_open($timezone));
      $offset = date_offset_get($date);
      $datetime = strtotime($event_instance->date->value) + $offset;

This is very painful DX and should be corrected at the module level. Storing a full ISO date string or a Unix timestamp are both valid options.
Reference: https://www.php.net/manual/en/datetime.format.php

agentrickard → created an issue.

Approved!

I have updated the MR. Here is the patch version.

It looks like setting NULL fails, but empty string works.

$values[$delta]['value'] = !empty($value['value']) ? (new \DateTimeImmutable($value['value']))->format('H:i:s') : '';

It fails to save because both fields are registered as REQUIRED in Drupal\field_time\Plugin\Field\FieldType::propertyDefinitions()

  public static function propertyDefinitions(FieldStorageDefinitionInterface $field_definition) {
    $properties['from'] = DataDefinition::create('string')
      ->setLabel(new TranslatableMarkup('Start time'))
      ->setDescription(new TranslatableMarkup('Format HH:MM:SS'))
      ->setSetting('maxlength', 8)
      ->setRequired(TRUE);

    $properties['to'] = DataDefinition::create('string')
      ->setLabel(new TranslatableMarkup('End time'))
      ->setDescription(new TranslatableMarkup('Format HH:MM:SS'))
      ->setSetting('maxlength', 8)
      ->setRequired(TRUE);

    return $properties;
  }

agentrickard → created an issue.

agentrickard → created an issue.

dcam → credited agentrickard → .

Right. For the first time in 18 (!) years, I am not using this module on client work, so I don't have time to maintain it.

@mably has been added with vcs perms and the permission to create releases.

@danrod has been added with vcs perms

I have added you as a maintainer with commit, issue management, and release priveleges.

volkswagenchick → credited agentrickard → .

Oddly, I can no longer reproduce this on Drupal 11.1.3

agentrickard → created an issue.

This has been fixed now.

Merged and left open for the bot.

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

Well, I just noticed that we are getting test failures on the bot patch. So those need to be looked at.

I will go ahead and roll and official 2.0.2 and then we can see what else needs to be done.

Thank you!

All that aside, this seems to be working as expected.

It looks like 📌 Automated Drupal 11 compatibility fixes for cacheexclude Needs review only requires a change to the info file.

If you can test that issue and confi9rm that it works on Drupal 11, I can roll a point release.

I have made some comments on the new MR. In general, I think that if you are enforcing sections, then the field should be required, which is an easier way to fix this issue on a site.

Even with the new MR, this still needs:

* A test
* Some code cleanup

There is a lot to digest here and it definitely means a 2.1.x branch.

IMO we should target this for Drupal 11.2 which is currently scheduled for June 2025

References related to changes in this PR:

* https://symfony.com/doc/current/service_container/autowiring.html
* https://phpstan.org/writing-php-code/phpdocs-basics#prefixed-tags
* https://php.watch/versions/8.1/readonly
* https://www.drupal.org/about/core/policies/core-release-cycles/schedule#... →

I don't think this is the right place to make this change, as it changes AccessControlHierarchyInterface (though that is not included in the patch).

If you have a one-site use-case, the recommended solution is to extend the Plugin with your own replacement and use that instead. See the Access Control section of the README.

This article (which includes some salty language) shows how to do that.

https://medium.com/@djphenaproxima/how-to-bend-drupal-8-plugins-to-your-...

Converts the JS to vanilla JS and removes all dependencies.

To test on Tugboat

1. Configure an editorial scheme using the Workbench Access vocabulary
2. Go to the Users page for a section (e.g. "/admin/config/workflow/workbench_access/section/sections/workbench_access/users")
3. Click the "Switch between textarea/autocomplete" link under the autocomplete / text area
4. On click the visibility of the autocomplete / text area should toggle

I am still not used to comments on MRs, so tickets need updating,.

I need to take a look.

Needs a new review.

Tagging with a related issue which may be the solution.

Marking for review

No reviews? Likely going to commit.

I wonder if this is a better fit for Workbench Menu Access?

If we left it here, I would suggest that we also do that for Taxonomy overviews and make it part of the plugin API.

Marking for review

Marking for review.

Marking for triage and review.

Committed to 2.0.x

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

Yeah,. I thought we could set that to strict. I suppose we can only skip them.

This appears to be a custom theme issue.

agentrickard → created an issue.

Odd, I would have expected the MR to not have a green check in that case.

Anyway, fixed.

MR is ready for review.

This is working fine on a clean install, but when applied to an existing Drupal 11 site, the `Use header` option still appears.

Investigating.

Patch no longer applies cleanly.

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

Here's a patch. I will fire up a MR in a minute.

However, it looks like this path only treats the symptom. The real issue is that in a preview context, some of the expected variables seem to be missing.

Also seems to occur in 3.0.x

agentrickard → created an issue.

agentrickard → changed the visibility of the branch 3505640-create-drupal-11 to hidden.

agentrickard → changed the visibility of the branch 3505640-create-drupal-11 to hidden.

agentrickard → created an issue.

Flagging, as I am interested in picking this back up, time permitting.

I do not think that is valid. It is an abuse of the API. If anything, it should be caught upstream before hook_node_grants is called.

Also, the link to the code does not seem relevant to this issue. That code seems to have moved https://git.drupalcode.org/project/search_api/-/blob/8.x-1.x/src/Plugin/...

Linking to parent issue.

Odd, the package redirecting to the new host -- https://packagist.org/packages/squizlabs/php_codesniffer

    "require": {
        "php": ">=7.2",
        "ext-mbstring": "*",
        "dealerdirect/phpcodesniffer-composer-installer": "^0.7.1 || ^1.0.0",
        "sirbrillig/phpcs-variable-analysis": "^2.11.7",
        "slevomat/coding-standard": "^8.11",
        "squizlabs/php_codesniffer": "^3.11.3",
        "symfony/yaml": ">=3.4.0"
    },

agentrickard → created an issue.

@siramsay, I assume you are talking about this part "Create the directory core/themes/olivero/components/chip/"?

Why don't you go ahead and update that reference to assume an `example` theme installed in `themes/contrib/example`.

larowlan → credited agentrickard → .

I added an MR -- https://git.drupalcode.org/project/simple_sitemap/-/merge_requests/119 -- and updated tests.

Something is a bit off in the test. I can't seem to get the sitemap to re-render. See this part of the test code:

    // Test sitemap result -- Not working as expected.
    # $this->generator->rebuildQueue();
    # $this->generator->generate(QueueWorker::GENERATE_TYPE_BACKEND);
    # $this->drupalGet($this->defaultSitemapUrl);
    # $this->assertSession()->responseContains('node/' . $new_node1->id());
    # $this->assertSession()->responseNotContains('node/' . $new_node2->id());

This returns a 404 instead of the sitemap. I assume that I missed a step somewhere.

Here's a version of the patch against 4.x. It does the following:

* Removes the form alter that disables per-entity overrides.
* Adds a method getAllowedEntities() to the EntityManager service
* Call that method from the EntityUrlGenerator in cases where the bundle is not auto-indexed.

Based on my reading of the code, access checking is handled by EntityUrlGeneratorBase, so that is omitted.

This likely needs a test, though is seems to work in my use-case.

If you would prefer an MR, I can make one.

I can confirm the same problem on Platform.sh (Solr 9.6) using the provided configset from the module.

Putting the provided configset in .platform/configsets/solr9/ throws the error.

https://solr.apache.org/guide/solr/9_6/configuration-guide/config-sets.h... suggests that the files should be in a nested /conf directory inside the configset.

When we move the files to a nested /conf directory (.platform/configsets/solr9/conf), the initial error is replaced by "Your config-set contains manually added customizations. Be aware that these will be lost when the config-set needs to be regenerated."

For the OP, I ran into this as well and could not get Brightcove's iFrame to work, so I ended up with this:

In mytheme.theme.

/**
 * Implements hook_preprocess_brightcove_player_responsive()
 *
 * We cannot render Brightcove inside CKEditor.
 */
function mytheme_theme_preprocess_brightcove_player_responsive(&$variables) {
  $variables['preview'] = FALSE;
  $route = \Drupal::routeMatch();
  if ($route instanceof RouteMatchInterface && $route->getRouteName() === 'embed.preview') {
    $variables['preview'] = TRUE;
  }
}

In mytheme/templates/brightcove/brightcove-player-responsive.html.twig

{% if preview %}
  {% set img_url = '/' ~ directory ~ '/imgs/video-thumbnail-.jpg' %}

  <div class="brightcove-player sizing-responsive">
    <div style="max-width: {{- max_width ~ units -}};">
      <img class="video__thumbnail" src="{{ img_url }}">
    </div>
  </div>
{% else %}
  <div class="brightcove-player sizing-responsive">
    <div style="max-width: {{- max_width ~ units -}};">
      <video-js class="vjs-fluid"
                controls=""
                data-account="{{- account -}}"
                data-embed="{{- embed -}}"
                data-player="{{- player -}}"
                data-usage="{{- data_usage -}}"
                data-{{- type -}}-id="{{- id -}}"
      ></video-js>
    </div>
    {% if is_playlist %}
      <ol class="vjs-playlist"></ol>
    {% endif %}
    <script src="//players.brightcove.net/{{- account -}}/{{- player -}}_{{- embed -}}/index.min.js"></script>
  </div>
{% endif %}

agentrickard → created an issue.

agentrickard → created an issue.

This MR needs to pull in the changes from 📌 Fix failing test Fixed

Latest patch is working nicely on Drupal 10.3.6.. Both import and export of a Layout Builder page work as expected.

This needs testing by people outside our project.

So you filed an MR that requires 10.3+ and didn't note that at all until after it was committed?

The current published release is now 1.6 and tests on GitLab CI with 10.3.

I will also note that I reviewed the module and it has no API and no clear means for adding items.

I would wait until it is stable and those are provided, as they are in Toolbar now.

I am going to remove the update hook because it has no added value to a site.

Any site that had an error due to basic_html would have already corrected it. New installs should be fine with the new config.

agentrickard → created an issue.