Portland, OR USA
Account created on 21 February 2008, about 16 years ago
#

Merge Requests

Recent comments

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Updated remaining tasks in issue summary (all tasks done).

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

This is ready for review.

I've converted the 2 test plugins, EmptyHelpSection and TestHelpSection to use the HelpSection attribute.

And I removed a trailing comma from the last item in the attribute HelpTopicSection. I guess we're not doing trailing commas for last items in these arrays in attributes? Yet, anyway. I think that code standards for these are still pending (that's my current understanding). Regardless, this makes everything consistent and is consistent with the Action example as well.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Thanks @quietone for the reminder! I am working on this now.

Also, I updated the IS with remaining tasks (and those already done).

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

There was a missing use statement for the HelpSection attribute class in HelpSectionManager.php. Adding this fixed the error on installation (because it fixed the value of the HelpSection::class parameter passed in the parent constructor, which was meaningless without the use Drupal\help\Attribute\HelpSection; statement.

I tested this on a successfully-installed 11.x-dev site and verified that HelpSection plugins displayed as expected on admin/help. Screenshot attached with HelpSection plugins provided by Help module highlighted.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

I'm working on this again. When I try to install Drupal 11.x-dev with this code running, the installation is failing with a typehint error in this new code.

TypeError: Drupal\Core\Plugin\Discovery\AnnotatedClassDiscovery::__construct(): Argument #4 ($annotation_namespaces) must be of type array, string given, called in /var/www/html/core/lib/Drupal/Core/Plugin/DefaultPluginManager.php on line 305 in Drupal\Core\Plugin\Discovery\AnnotatedClassDiscovery->__construct() (line 56 of core/lib/Drupal/Core/Plugin/Discovery/AnnotatedClassDiscovery.php).

After fixing this, I will test the HelpSection in the UI to make sure it's working and upload a screenshot of that.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Thanks for the review, @larowlan! I think I addressed everything, though there was 1 point of confusion about line 29 of core/modules/help/src/Plugin/HelpSection/HelpTopicSection.php, which I asked about in GitLab.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Looks like I missed some coding standards. I'll take a closer look tomorrow if I can. Happy to have help though.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

I took a stab at this.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Amber Himes Matz โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Updated title to fix typo in "annotations" to ensure issue is discoverable in searches for "annotations".

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Adding tag per Gรกbor's request.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

@fjgarlin Is there anything we can do to help hasten the launch of the new API site? Are there any project issues that are blocking it that we could help with? My colleagues and I would welcome opportunities to help, as we have so many API links on Drupalize.Me that are breaking, and we get weekly support messages from our members telling us about the broken URLs. It's a bit of a maintenance pain, as we try to keep the links up to date with the latest version of Drupal, and now I'm having to change some of them back to "9". It's confusing for folks.

We totally get the project priority thing, but if there is a way to help and get the new API site launched, we'd love to know about it.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Hiding patch files, as the MR contains the latest solution.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Thanks for the clarification, @andypost. This patch adds the patch from #2 (the changes to core/modules/help/tests/src/Kernel/HelpSearchPluginTest.php). Interdiff included.

Ok, I think this is ready for review!

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Alright, here's a patch that adds to the one in #3, updating the URL and link text to the Help Topic Standards docs page (without the redirect).

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Replaced "Drupal 8" with "Drupal" in several places.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Updated the IS. Per @andypost in #99 (and I, as co-maintainer, concur), moving some items in the IS that were in the section "Needs discussion" to "Not part of the roadmap".

  1. Moved โœจ Do we need hierarchy/order for help topics? Active to the section "Not part of the roadmap".
  2. Moved "
  3. #3064854: Allow Twig templates to use front matter for metadata support โ†’ , and if that is done, #3085972: Replace Drupal\help_topics\FrontMatter with Drupal\Component\Utility\FrontMatter โ†’
  4. " to the section "Not part of the roadmap"

  5. Moved ๐Ÿ“Œ Move testing of help topic rendering into child of GenericModuleTestBase Active to "Not part of the roadmap" as it's not a part of this roadmap, but a child of ๐ŸŒฑ Deprecate hook_help() and combine with Topics Active (per #111

Hopefully the IS now clearly reflects that there are no action items remaining.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

A nice workaround could be if help topics could receive a context provided by some service, maybe defining injection in the frontmatter.

I imagine @fgm is not the only one to need this. Sounds like "Allow help topics to get context defined in frontmatter" should be opened as a child issue of this one? @fgm do you want to open the issue, or should I? In either case, I'm interested in more details as to what you are looking for, and if you could provide examples of your modules that use context in their hook_help() implementations. Thank you.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

LGTM and credit has been added already, so closing as fixed. Thanks all!

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Thank you @quietone for pointing out that this IS is a bit of a hard-to-understand mess. And to @xjm for pointing out the scoping problem. I have updated the title and issue summary as follows:

  1. Removed statement about it being postponed, because it is not.
  2. Updated title to "Fix up minor copy problems in help topics" since this issue is only focused on copy edits, not content or code errors (scope).
  3. Updated issue summary. Moved the original summary down to "Original issue summary section". Organized the IS into template with clear action items and scope, and indicated what has been done.
  4. Added referenced issues
  5. Added Release notes snippet

Also, just to add additional context and clarification, this issue historically was a sort of meta issue to collect problems. Problems were fixed in other issues or new issues created. This issue is now scoped to minor copy edits related to word usage and consistent link text.

I hope this update makes it easier to review and commit. But feedback is always welcome on how it could be further clarified if clarification is required. Forgive us if we are a bit strained as getting help topics stable has been a long long road.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

When I checked the MR just now, it said it was still blocked even after @andypost rebased earlier today, so I hit the rebase button again, and now it looks "able to merge" again.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

I found this issue via #bugsmash, where catch requested reviews and testing, so adding the Bug Smash Initiative tag.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

I validated the issue in Drupal 11.x in Firefox 114.0.2 and Chrome Version 114.0.5735.198 using DDEV by following the steps to reproduce in the IS, and found 2 JS assets' response headers included x-generator: Drupal 11 (https://www.drupal.org), as described in the issue summary problem. (Screenshots attached.)

To test the patch, I applied the patch, cleared the cache on the Performance page, and returned to the Add article page. I opened the devtools' Network tab, and refreshed the page 3 times. I inspected the response headers for each aggregated CSS and JS asset, and did NOT find x-generator: Drupal 11 (https://www.drupal.org) as before.

I navigated to other admin and front-facing pages on the Umami site, and inspected response headers for aggregated CSS and JS assets. Where I found x-generator: Drupal 11 (https://www.drupal.org), I refreshed the page, and noticed it was then gone. (With the patch applied.)

As far as I can tell, this patch in #23 addresses the problem. Setting to RTBC.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA
๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Looks good to me! Thanks @andypost for taking care of this.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Updated issue summary regarding opening ๐Ÿ“Œ Standardize how external links are wrapped with Twig trans/endtrans tags in help topics Active as separate issue and not addressed in this issue.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

I've pushed new commits that "undo" changes to Twig trans tag wrapping and resolved the thread discussing this. Ready for review (assuming tests pass).

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

After discussing with @andypost in Slack, I'm going to "undo" changes in this MR where I moved the Twig {% trans %}{% endtrans %} wrapping (in Additional resources external links). I opened a new issue to discuss this ๐Ÿ“Œ Standardize how external links are wrapped with Twig trans/endtrans tags in help topics Active .

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

I've submitted a merge request which addressed all remaining "PATCH" items in the issue summary. These are mostly nits and standardizing on the User Guide links in the Additional resources.

I've opened separate issues where there a content update to a topic is needed.

This will be ready for review after the tests run (and hopefully pass).

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

I've updated the issue summary to denote where items have been fixed, outdated (no longer an issue), "won't fix", moved to separate issues, or in-scope ("PATCH") for this issue.

Also changed this issue back to "Task" as I have a patch/MR almost ready to fix the remaining in-scope issues here.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

From this issue summary, I added the following item:

On #3150364: Add a description for the language toolbar button to the CKEditor help page โ†’ they also made a list of accessibility features and a section about making accessible text. I'm not sure we covered that well in our topic on Accessibility, so we should check. See this screenshot of the output of one of the patches:

to ๐Ÿ“Œ Create or update help topics that cover CKEditor 5's module overview text in hook_help() Active

See also this comment: https://www.drupal.org/project/drupal/issues/3358585#comment-15116862 ๐Ÿ“Œ Create or update help topics that cover CKEditor 5's module overview text in hook_help() Active

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Added another CKEditor help related item from ๐Ÿ“Œ Fix up minor copy problems in help topics Fixed to the issue summary.

On #3150364: Add a description for the language toolbar button to the CKEditor help page โ†’ they also made a list of accessibility features and a section about making accessible text. I'm not sure we covered that well in our topic on Accessibility, so we should check. See this screenshot of the output of one of the patches:

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Before you โ€œrerollโ€: Please remember that the โ€œNeeds rerollโ€ tag also applies to merge requests that need rebasing. I understand how this can be confusing. And, adding patches to an issue using merge requests can result in confusion and lost work. Please avoid doing that. Refer to the documentation on how to rebase a merge request:

https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... โ†’

Comparing the MR in #8 that was set to needs review and the patches that followed, itโ€™s clear that all of the patches that followed were incomplete โ€” missing files and changes that were included or requested in the MR. (This is why we donโ€™t want to switch between MR and patch workflow if it can be helped.)

Hiding all of the files since they derailed the issue (except for the ones showing the CSS class name changes in the inspector since Iโ€™m not sure how helpful those are or not). The MR (link in #8) is where work needs to resume from:

1. The MR needs to be rebased. The target branch probably needs to be updated as well.
2. The MR needs review โ€” hint: the @file docblock in wide-content.pcss.css needs to conform to https://www.drupal.org/docs/develop/standards/php/api-documentation-and-... โ†’ with a summary and description with lines between.
3. When comments on the MR have been addressed, they should be marked as resolved. (The current unresolved comment was addressed, but the update doesnโ€™t meet @file docblock coding standards.)

Please refrain from adding screenshots of terminal windows showing commands for applying a patch. The CI jobs and test runs will tell us if a code change cannot be merged. Screenshots are really only useful for showing a UI change or problem.

Removing credit where patches were added unnecessarily.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

There appear to be a number of unresolved threads on the MR that will need to be resolved before this can be RTBCโ€™d again. Thank you!

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

@Gauravvvv Thanks for your attention to the โ€œNeeds rerollโ€ tag, but this issue appears to be using an MR-based workflow, not a patch workflow. I realize that the โ€œNeeds rerollโ€ tag could be confusing, when what is needed is a rebase on the merge request, not a patch file. Please refer to the docs on rebasing a merge request: https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... โ†’

Also, it does look like there are a number of unresolved threads in the MR that should be marked as resolved if the feedback has been addressed.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

@eojthebrave asked if I would take a look at the copy and check for spelling/grammar. This is kind of awkward to do because weโ€™re not dealing with a file or patch. So hereโ€™s the whole blob with my edits included.

In the prereqs, does a module need to be installed if it contains a SDC? It just says the theme must be enabled (which I changed to the word, โ€œinstalledโ€. We should include both cases, I think, if thatโ€™s the case.

A component is a piece of the user interface (UI) that has its own logic and appearance. A component can be as small as a button, or as large as an entire node. Drupal's Single Directory Components consist of metadata that describe the component, HTML markup, and, optionally, CSS and JavaScript, which are all located in the same directory. Hence, the name, Single Directory Components.

To demonstrate how Single Directory Components work, we're going to walk through creating an example 'chip' component and use it in a theme.

## Prerequisites to working with components

- You must be using Drupal 10.1 or greater
- You must have the Single Directory Components module enabled (at _admin/modules_)
- You must have a theme (or module) that you want to add components to. The theme must be installed (at _admin/appearance_)
- You must know where in the filesystem the theme's code is located. (We'll be using the Drupal core Olivero theme, located at _core/themes/olivero/_ in this example.)

## Define a component

Every component requires a single directory for its assets to reside in. This directory must exist in a _components/_ subdirectory of your theme (so that Drupal can find it). A _{NAME}.component.yml_ file with metadata about the component is required.

Pick a name for your component. We'll use "chip" for our example. It should be unique within the theme. Components are namespaced, so multiple different themes could declare a "chip" component.

Create the directory _core/themes/olivero/**components/chip/**_. Then, in that directory, create a file starting with your component name,  followed by _.components.yml_. For example, _chip.components.yml_.

Populate the _chip.components.yml_ file with metadata that describes the component:

```yaml
name: Chip
props:
  type: object
  required:
    - color
  properties:
    # Can the chip be dismissed by clicking on it?
    dismissable:
      type: boolean
    # One of 'primary', or 'secondary'.
    color:
      type: string
slots:
  # Content to display in the chip.
  chip_content: {}
```

The data in this file describes your component to Drupal and to anyone who wants to make use of it. This one includes a human-readable `name` for the component. Definitions of the componentโ€™s inputs in the form of **props** (for data whose type and structure is well defined) and **slots** (for data with an unknown structure, like nested components). These act as an API or contract for the component, and is like saying, "We promise to always accept and use these inputs."

[Learn about additional metadata values](https://www.drupal.org/docs/develop/theming-drupal/using-single-directory-components/annotated-example-componentyml#s-full-example).

## Add some markup

A component that doesn't output anything isn't that useful. So letโ€™s add some markup via a Twig file. Within the componentโ€™s directory, create a _.twig_ file using the name of your component.

For example, in _components/chip/chip.twig_, add the following:

```html
{%
  set classes = [
  'chip',
  'chip--color-' ~ color|clean_class,
  dismissable ? 'chip--dismissable',
]
%}
<div class="{{ classes|join(" ") }}">
  {% block chip_content %}{% endblock %}
</div>
```

The above code contains some HTML markup and Twig expressions that make use of component inputs in order to output dynamic content. Note: the Twig variable names (and their values) map to the props and slots defined in your _{NAME}.component.yml_ metadata file.

## Use your component in a template file

In order to use the markup generated by your component, you'll need to embed it into a Twig template file in your theme. We're going to add ours to the node template file, because we're going to display the node type (bundle) as a chip.

Edit the _core/themes/olivero/templates/content/node.html.twig_ template file and add the following (where you want the chip to display):

```html
{% embed 'olivero:chip' with {
  color: 'primary',
  dismissable: true
} only %}

  {% block chip_content %}
    Type: <span>{{ node.bundle() }}</span>
  {% endblock %}

{% endembed %}
```

This Twig code embeds the chip component into the Twig template file and defines the values to pass to the componentโ€™s inputs. This uses standard Twig embed syntax. Some things to note: the namespacing of the component `olivero:chip` resolves to the 'chip' component in the _olivero_ theme. And, the definition of the input props (`color` and `dismissable`) and slots (`chip_content`) correspond to the props and slots you defined in your componentโ€™s metadata file (_{NAME}.component.yml_).

Clear the cache and view a node on your site. You should see the node type output as a `chip` on the page in the place you embedded the component in the node template file.

## Add some style

Let's add some CSS and make that chip look a little more attractive. This requires creating a CSS file with the name of your component in the component directory, and well, ...that's it!

Create the file, _core/themes/olivero/components/chip/chip.css_, with the following contents:

```css
.chip {
  display: inline-block;
  padding: 0.25rem;
}

.chip--color-primary {
  background-color: var(--color--primary-50);
  color: white;
}

.chip--color-secondary {
  background-color: var(--color--gray-100);
  color: var(--color-text-primary-medium);
}

.chip--dismissed {
  display: none;
}
```

If the componentโ€™s _{NAME}.css_ file exists, Drupal will automatically find it, and include it on the page. In the case of our chip component, since the _chip.css_ file exists, Drupal will use it whenever the chip component is used in a template file.

## Finally, add some interaction with JavaScript

Adding some JavaScript to our component isn't any harder. Let's add some JavaScript that will make it so the chip can be dismissed (hidden) when a user clicks on it. This requires creating a _{NAME}.js_ file with the component name in the componentโ€™s directory.

Create the file, _core/themes/olivero/components/chip/chip.js_, with the following JavaScript code:

```js
((Drupal) => {
  Drupal.behaviors.chip = {
    attach(context) {
      context.querySelectorAll('.chip--dismissable').forEach((chip) => {
        chip.addEventListener('click', () => {
          chip.classList.toggle('chip--dismissed');
        })
      });
    },
  };
})(Drupal);
```

Refresh the page and Drupal should automatically locate this new JavaScript file and include it whenever the chip component is used. You can test that it's working by setting the `dismissable` prop of your component to `true`, and then clicking on the chip element. (It should disappear.) Now set it to `false`, and clicking should have no impact.

Note: this ties together code in CSS where we have classes to show/hide the chip and the Twig file where we dynamically decide whether or not to add the `.chip--dismissable` class depending on the `dismissable` prop's value.

## Take it further

Components can do a lot more. Now that you understand this brief example, check out the [complete annotated example](https://www.drupal.org/docs/develop/theming-drupal/using-single-directory-components/annotated-example-componentyml#s-full-example) to learn more. Here are a few things to experiment with:

- You can include other assets in the directory, too, like images, but they won't be automatically loaded. You'll need to reference them via your Twig, CSS, or JavaScript files. Can you add an icon to your chip component?
- Use the `libraryOverrides` in your _{NAME}.component.yml_ file to require additional JavaScript libraries like _drupal/once_ or jQuery. Can you rewrite the JavaScript so that it uses the _drupal/once_ library?
๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

1.

+++ b/core/lib/Drupal/Component/FileSecurity/FileSecurity.php
@@ -157,7 +157,7 @@ protected static function writeFile($directory, $filename, $contents, $force) {
+    if (@file_put_contents($file_path, $contents ."\n")) {

Coding standards nit. Should be a space after the concatenation dot.

2. And, it needs another reroll.

3. @mstrelan -- Can you give reviewers a clue on how to manually test your patches? Looks like there's some confusion there.

4. I'll add a Drupal 10 test run for the other patch (1915772-34-generated-htaccess-new-line.patch) in #34.

5. Hiding some files. It's confusing because we're dealing with 2 files. (I'm not entirely clear on why that is -- maybe because one is dynamically generated.)

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Hiding old patches, as it looks like the patch from #52 is the one!

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

1. Looks like there is 1 unresolved comment (that got addressed but was never marked as resolved): https://git.drupalcode.org/project/drupal/-/merge_requests/3075#note_137045)

2. The merge request is currently blocked and needs a rebase.

Thank you!

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

The MR is currently blocked and needs to be rebased. Also, the version on the issue is 11.x-dev but the target branch on the MR is 10.1.x, so maybe the target branch on the MR needs to be updated as well? Just a bit of git-related cleanup required before this can be reviewed by a committer. Thank you!

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Hiding the patch file because it looks like the workflow was switched to MR.

The MR is currently blocked according to GitLab and needs to be rebased.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Looks like the MR was updated after the last RTBC and needs another review. Thank you!

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Looks like this MR needs to be rebased onto 9.5.x, as the merge is currently blocked according to the MR in GitLab. (Clicking the Rebase button didn't work for me.)

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

It's really unclear if this issue is using a patch or a merge request, and which one contains the fix. Please clarify where the work is being done on this issue. (And contributors, please follow that guidance.)

Whether patch or MR is chosen, the current ones don't apply. So after a path is chosen (patch or MR), reroll/rebase.

Also, posting screenshots of a terminal window applying the patch is not helpful. I've hidden the screenshot file showing terminal window. Screenshots are helpful if a patch affects a change in the UI itself. Please review https://www.drupal.org/community/contributor-guide/task/add-screenshots-... โ†’ -- especially the "Goal".

Thank you!

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Looks like this MR needs a rebase, as the merge is blocked. It might work to just click the Rebase button in the MR link.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

We triaged this as part of Bug Smash Initiative. This needs more info, especially an issue summary update. It would be great if someone could go to #1818574: Support config entities in typed data EntityAdapter โ†’ , and starting with comment #73, figure out what needs to be done in this issue. Also, changed to a task.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Thank you for catching the typos, @rgladden!

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

@webchick Thank you so much. Iโ€™ll never forget how the โ€œwebchickโ€ tour Lullabot workshop for Drupal 7 happened at exactly the time when I was trying to get back on my feet and back into Drupal development after a major life setback. Thatโ€™s when I first met you in person. You were such an inspiration to me and I proudly wore my webchick tour t-shirt until it was full of holes and falling apart at the seams.

Your mentorship, guidance, and encouragement during the development of Help Topics as an experimental module was so vital and helpful to me as I was struggling to learn how to become a maintainer myself.

But more than that, you are just such a terrific human being. I miss your presence in the community and I wish you the very best in all your future endeavors.

:hugs:

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

The link is now fixed. Thank you @apaderno! Thereโ€™s nothing to commit here since it was a documentation page edit.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Thanks for the broken link report! Technically, I think an issue for documentation pages like this one go in the https://www.drupal.org/project/issues/search/documentation โ†’ queue, but that queue isn't being actively managed it looks like, so we can leave it in here for now.

I'll see about pinging an admin of that page to get it updated.

And, I'm setting the status to Active since there's no patch or work in progress on it.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Sorry, should have marked this as Fixed.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Sorry for the noise, I should have marked these as "Fixed".

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Looks good and that the credits are complete.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Looks good and I reviewed the credits, too.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Looks good and I double-checked the credits with the "who is here" section.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

We decided in #3037230-26: Finalize the merge of Help Topics into Help โ†’ that it makes sense to incorporate the changes in that MR into this one, since they affect several of the same files. So I am doing that.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Yes, I think it makes sense to incorporate the changes here into #3087499 instead.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Ok, looks like it's not necessary (or appropriate) for me to add this to the 11.x branch, since it's a minor bug fix that can go in to 10.1.x (hopefully). The tests are passing now after I rebased again, so this looks good for a final review.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

First, I squashed the 3 commits and rebased 10.x.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

I'm working on rebasing onto 11.x.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

We triaged this in Bug Smash.

Is this still an issue? It would be great to add a failing test to prove the issue.

Also adding tags for re-roll and tests.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Thanks for reporting this issue. We rely on issue reports like this one to resolve bugs and improve Drupal core. This issue is being triaged as part of the Bug Smash Initiative.

I donโ€™t think youโ€™re โ€œmagicallyโ€ creating a new template suggestion by implementing function MYMODULE_preprocess_node__event(&$variables) {}. Youโ€™re implementing a hook that is covered by the core node module, which provides default template suggestions, including dynamic ones based on the machine name of the content type (aka โ€œbundleโ€).

See core/modules/node/node.module (โ€œbundleโ€ is the machine name of the content type):

/**
 * Implements hook_theme_suggestions_HOOK().
 */
function node_theme_suggestions_node(array $variables) {
  $suggestions = [];
  $node = $variables['elements']['#node'];
  $sanitized_view_mode = strtr($variables['elements']['#view_mode'], '.', '_');

  $suggestions[] = 'node__' . $sanitized_view_mode;
  $suggestions[] = 'node__' . $node->bundle();
  $suggestions[] = 'node__' . $node->bundle() . '__' . $sanitized_view_mode;
  $suggestions[] = 'node__' . $node->id();
  $suggestions[] = 'node__' . $node->id() . '__' . $sanitized_view_mode;

  return $suggestions;
}

The way to create a new template suggestion, or to alter the suggestions provided by another module or theme, is to implement HOOK_theme_suggestions_alter() or HOOK_theme_suggestions_HOOK_alter(). You can use either of these hooks to add/remove suggestions from the list or reorder the list by making changes to the $suggestions array.

I see a couple of problems with the original report that makes me think that maybe this is more of a misunderstanding of how the system is designed to work, rather than a bug with the system:

1) Implementing a specific hook for a template and then not creating that template seems counterproductive.
2) There is already a hook in Drupalโ€™s API to reorder theme hook suggestions provided by any module or theme and thatโ€™s hook_theme_suggestions_alter() or itโ€™s more specific counterpart, hook_theme_suggestions_HOOK_alter()

I could be totally wrong about this, so feel free to re-open and provide more information or context.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

The MR was rebased to 11.x, so updating the version metadata in the issue to 11.x-dev.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

I've pushed a fix after working out a solution. I've updated the issue summary. This is ready for review and testing.

Steps to test:

  1. Use the Tugboat link in this issue to review the merge request.
  2. Login as admin/admin
  3. Install Help Topics (admin/extend)
  4. Navigate to admin/help
  5. Click on the top-level topic, "Managing contact forms"
  6. Under "Related topics", click on "Setting a default contact form" (admin/help/topic/contact.setting_default).
๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Also, I rebased the MR to 10.1.x, but maybe the target branch needs be update to 11.x (I'm not sure).

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

I reviewed the merge request via the generated Tugboat instance and I see that the HTML didn't get rendered properly, as you can see here (login: admin/admin)

https://mr3958-kxepnyr9mnydxhfgeg2mnr0rewij1yfl.tugboatqa.com/admin/help...

I think this is because there's HTML in the link text, so it gets escaped when passed through render_var(help_route_link()).

I'll do some testing and figure out a better suggestion.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

I've added comments and suggestions in GitLab, in the merge request.

๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

Thanks for testing @smustgrave. I haven't had a chance to complete my review of the grammar of newly added comments, as @andypost requested. (I have found 1 error so far.) I'll update the issue to Needs work after my review is completed.

Production build https://api.contrib.social 0.61.6-2-g546bc20