๐Ÿ‡บ๐Ÿ‡ธUnited States @gslexie

Account created on 22 July 2014, over 10 years ago
#

Merge Requests

More

Recent comments

๐Ÿ‡บ๐Ÿ‡ธUnited States gslexie

Looks like the update to admin_toolbar_tools has been merged into 3.5, so this issue is probably either fixed or outdated

๐Ÿ‡บ๐Ÿ‡ธUnited States gslexie

I stumbled across this while searching for something else and I'm pretty sure this issue has been resolved in Core 10.2 by https://www.drupal.org/node/296693 โ†’ . There is also a submodule included in admin_toolbar called admin_toolbar_links_access_filter that was supposed to workaround the issue for earlier versions.

๐Ÿ‡บ๐Ÿ‡ธUnited States gslexie

FWIW I don't think it would make the most sense for a Core module to be altering menu links from a contrib. So I created an issue and a patch for admin_toolbar to try to get it fixed there. There's already a deriver that conditionally adds links for various optional modules, so it fits neatly in there.

(see https://www.drupal.org/project/admin_toolbar/issues/3457902 โœจ Forward Compatibility with Project Browser Active )

๐Ÿ‡บ๐Ÿ‡ธUnited States gslexie

There are likely a number of pathways to reach the same error message. But I've figured out how I did it with my own code:

I programmatically created a custom block plugin. My content team has created a lot of nodes with layout overrides, and my custom block has been included on a number of those nodes. I am now refactoring the block, which involves adding a required context definition (layout_builder.entity).

Layout Builder serializes the entire block config and writes it into the database at the time the block is created, and that includes the block's context mapping. Previously created blocks are missing the necessary mapping and *sometimes* create this error message. Specifically, it crashes while editing a layout override but works as I intended while viewing a node at its entity.node.canonical route. Somehow newly created blocks are acquiring the necessary mapping automatically and they work in both cases.

I solved the issue by defining a default mapping in my block plugin as described at the end of this stackoverflow thread: https://drupal.stackexchange.com/questions/283983/how-to-create-a-custom...

  public function defaultConfiguration(): array {
    return [
        'context_mapping' => [
          'entity' => 'layout_builder.entity',
        ],
      ] + parent::defaultConfiguration();
  }

It's not yet clear to me whether I should be needing to define that mapping explicitly, seeing as Layout Builder is able to provide it automatically in certain situations. But it has resolved my issues, and I wonder if some contrib modules that contain custom block plugins may have the same problem.

๐Ÿ‡บ๐Ÿ‡ธUnited States gslexie

gslexie โ†’ changed the visibility of the branch 3310908-consider-making-detail to active.

๐Ÿ‡บ๐Ÿ‡ธUnited States gslexie

gslexie โ†’ changed the visibility of the branch 3310908-consider-making-detail to hidden.

๐Ÿ‡บ๐Ÿ‡ธUnited States gslexie

gslexie โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡บ๐Ÿ‡ธUnited States gslexie

gslexie โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡บ๐Ÿ‡ธUnited States gslexie

This came up again on a new project and I finally found some time to take another look. I've added a new MR that passed all tests locally, with a combination of changes from the patches in #9 and in #20, and my own adjustments:

NodeFormMenuTest:

  • Added a test that previously assigned menu links won't be forcibly removed even if the editor doesn't have direct access to the parent section.
  • Fixed a prior test that looks to be intended to create a new menu link, but doesn't (I need this link for my test).

Menu::alterForm:

  • A little bit of refactoring to (hopefully) simplify the main foreach loop
  • Removed the separate check for root menu item. As far as I can tell, this check is necessarily covered by the Remove unusable elements, except the existing parent. section already and probably exists for historical reasons. The tests pass without it.
  • Included from #20: hiding the menu UI where the user has no sections. BUT skip this if there is already a menu link previously assigned by another user (i.e., we are editing a previously created node).
  • Included from #9: Don't remove the default value, BUT only if it was already assigned by another user
  • Included from #20: the final fallback behavior
๐Ÿ‡บ๐Ÿ‡ธUnited States gslexie

I tested the following steps:

  • I installed a fresh Drupal instance with demo_umami profile
  • I added added drupal/insert_view to the project and enabled the module
  • Created a Content view with a single contextual filter on node ID, with a default value "Content ID from URL"
  • Add the insert_view filter the the basic HTML text format
  • Create a new basic page and add [view:test_view] to the body text
  • Run cron to rebuild the search index

Confirmed, when running cron to build the search index, I crash with the same TypeError cited here.

I then added the patch:

        "patches": {
            "drupal/insert_view": {
                "search": "https://git.drupalcode.org/project/insert_view/-/merge_requests/8.patch"
            }
        },

I can confirm that the patch resolved the error during cron run.

๐Ÿ‡บ๐Ÿ‡ธUnited States gslexie

As far as I can tell the latest coding standards do indeed allow for up to 120 characters. But, I do feel the changes to those arrays improves readability, so I'm considering to include the patch from #2 regardless.

๐Ÿ‡บ๐Ÿ‡ธUnited States gslexie

One way to work around this is to override the exposed form template (or maybe form_alter it), replace the clear button entirely with a custom button, and write a Javascript to zero out all form elements and simulate a click of the submit button.

E.G., this is my script. If you have more complex default values then you may need to customize the script for individual views, but at least it works with AJAX.

    Drupal.behaviors.CustomClear = {};
    Drupal.behaviors.CustomClear.attach = function (context, settings) {
        const buttons = once('custom-clear', '.views-exposed-form input[type=reset]', context);
        buttons.forEach(function(element) {
            const $button = $(element);
            $button.click(function() {
                const $form = $(this).closest('.views-exposed-form');
                $form.find('input[type=checkbox]').prop("checked", false);
                $form.find('select, input[type=text]').each(function() {
                    $(this).val('');
                });

                $form.find('input[type=submit]').click();
            })
        })
    };
๐Ÿ‡บ๐Ÿ‡ธUnited States gslexie

#9 solved the problem for me. Unfortunately #20 did not.

We still have problems because our users are not always assigned to a top-level page section. For a real world example, we have a "Student Life" section with a "Student Handbook" child page, and numerous subsections. Like such:

  • Student Life
    • Orientation
    • Student Services
    • Student Handbook
      • Section 1
      • Section 2
      • ...

Somebody has been assigned editing privileges to the entire Student Handbook section, but NOT the rest of Student Life.

While creating new pages, she can only insert nodes to the menu as descendants of Student Handbook. This is surely correct behavior.

While editing existing pages, she still can only insert nodes as descendants of Student Handbook. This is mostly fine, but the problem arises when she tries to edit the Student Handbook page itself. Since it is not a descendant of itself, she cannot keep it in place, which is pretty nonsensical.

I believe #9 is on the right track in stipulating that authors should never be forced to relocate existing nodes as a consequence of updating content.

It's even more problematic with Content Moderation enabled. Relocating a page in the menu is not permitted while creating or editing a draft (because menu links aren't revisioned at all). In this case, when Workbench Access forces the user to relocate the page it means she cannot create a draft at all!

#9 is also similar to the way Drupal Core handles text formats. While editing a node with a text field that already has an un-permitted text format assigned it's OK to leave the format in place but we simply cannot edit the text in that case. We aren't forced to remove the formatted that a higher privileged user has already written.

๐Ÿ‡บ๐Ÿ‡ธUnited States gslexie

I believe the list of "logo" and "short description" tasks was derived from the 100 most popular modules, so this must be referring to https://www.drupal.org/project/google_tag โ†’ (83000 installs) and not the other one (2600 installs). I found some outdated tasks that have a little more historical context, plus there is a link to the correct module: https://www.drupal.org/project/project_browser/issues/3282866 ๐Ÿ“Œ [Meta] GoogleTagManager Project updates for Project Browser Closed: duplicate

๐Ÿ‡บ๐Ÿ‡ธUnited States gslexie

document.getElementById('project-browser').backgroundColor

You might have issues with the execution order but I think you will also have problems identifying background colors that are defined on a parent element. E.g., the bodytag is white in Claro but #project-browser just has a transparent background.

I tinkered with this a little and came up with a fiddle that climbs the chain of parent elements to find a background color: https://jsfiddle.net/s8u5yxzL/4/

That method seems to work for most normal situations, but would still theoretically have issues with some cases like understanding background images, gradients or reliably guessing what's behind absolute positioned, or floated elements.

๐Ÿ‡บ๐Ÿ‡ธUnited States gslexie

This patch creates a distinct project_browser.detail route for the project detail pages. It has the unanticipated side effect of hiding the "Extend" local task group entirely while looking at the detail pages. This was initially off-putting, but I came to believe I was looking for the tabs only because I had been working on them specifically. With the tabs gone, the "Back to Browsing" link is much more prominent and useful and is no longer competing with the tabs for an end user's attention.

The approach is also consistent with other navigations in Drupal Core. For example, the Block Layout page is inside of a local tasks tabset, but the detail pages for configuring individual blocks are not.

It is tempting to move the module title into the Drupal page title after this change, but that would be functionally complicated because the existing plumbing is built to expose project data to Svelte through an API and not for direct use by PHP code. Moving the title may also lead to a cascade of other design changes.

๐Ÿ‡บ๐Ÿ‡ธUnited States gslexie

I wonder if we should come up with a convention for all of these jQuery UI library module. In mine, I particularly wanted to gently suggest that this a dependency to remain backward compatibility for older projects, but not recommended for new ones. But there are a couple that are already RTBC and each have a different format. And a number of others that are not yet written:

jQuery UI Datepicker ๐Ÿ“Œ jQuery UI Datepicker Project - Add a short description RTBC
jQuery UI ๐Ÿ“Œ JQuery UI Project - Add a short description RTBC

๐Ÿ‡บ๐Ÿ‡ธUnited States gslexie

Added a proposed short description derived from the first sentence of the full description.

๐Ÿ‡บ๐Ÿ‡ธUnited States gslexie

Attached patch to transform the URLs. This does 3 things:

  1. Replace the existing relativeToAbsoluteUrls function with a general purpose sanitization function, which calls the relative to absolute function and also calls a new function to transform drupal.org links to Project Browser links. This could also be extended with additional sanitization operations in the future.
  2. Adds the function drupalOrgToProjectBrowser to transform drupal.org links to Project Browser links. This logic was included in the ProjectBrowserSource plugin so that custom sources can potentially link to projects in other locations and can choose to apply their own logic for the correct behaviour of those links.
  3. Replace the logic in ModulePage.svelte that adds target="_blank" to all links with conditional logic in drupalOrgToProjectBrowser to add target="_blank" to all links except the newly transformed links, which will forcibly open in the same browser window instead.
Production build 0.71.5 2024