- Issue created by @larowlan
- πΊπΈUnited States Amber Himes Matz Portland, OR USA
Amber Himes Matz β made their first commit to this issueβs fork.
- Assigned to Amber Himes Matz
- Merge request !6590Adds HelpSection Attribute class, updates constructor, updates plugins β (Open) created by Amber Himes Matz
- Issue was unassigned.
- Status changed to Needs review
7 months ago 3:56am 14 February 2024 - πΊπΈUnited States Amber Himes Matz Portland, OR USA
I took a stab at this.
- Status changed to Needs work
7 months ago 4:04am 14 February 2024 - πΊπΈ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.
- Assigned to Amber Himes Matz
- Issue was unassigned.
- Status changed to Needs review
7 months ago 4:52am 14 February 2024 - πΊπΈ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.
- Status changed to Needs work
7 months ago 2:21pm 14 February 2024 - First commit to issue fork.
- Assigned to Amber Himes Matz
- πΊπΈ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.
- Issue was unassigned.
- Status changed to Needs review
7 months ago 11:24pm 14 February 2024 - πΊπΈ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 theuse 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.
- π³πΏNew Zealand quietone New Zealand
I glanced at MR and see that the test HelpSection plugins still need to be converted.
- Assigned to Amber Himes Matz
- Status changed to Needs work
7 months ago 4:15pm 15 February 2024 - πΊπΈ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).
- Issue was unassigned.
- Status changed to Needs review
7 months ago 7:02pm 15 February 2024 - πΊπΈ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
Updated remaining tasks in issue summary (all tasks done).
- πΊπΈUnited States smustgrave
See that all @HelpSection have been replaced and tests appear to be passing.
Applying locally I can still use help module just fine
+1 from me but will leave in review for additional eyes.
- Status changed to RTBC
7 months ago 10:25pm 23 February 2024 - πΊπΈUnited States smustgrave
Re-reviewed and appears all feedback has been addressed.
- First commit to issue fork.
- π«π·France andypost
+1 RTBC, just added
declare(strict_types=1);
to new class and rebased - Status changed to Needs work
6 months ago 2:57pm 1 March 2024 - π¬π§United Kingdom catch
This is missing deriver support per π Add an assert that ensures all attribute plugins support derivers Needs review
- Status changed to Needs review
6 months ago 3:24pm 1 March 2024 - πΊπΈUnited States smustgrave
@andypost mind taking a look at the phpcs error? Can review after.
- Status changed to RTBC
6 months ago 4:04pm 1 March 2024 - πΊπΈUnited States smustgrave
Still blows my mind you can make that change and few minutes later tests are done.
Feedback has been addressed.
Thanks!
- πΊπΈUnited States Amber Himes Matz Portland, OR USA
Updated issue summaryβs remaining tasks about adding the optional deriver argument, and crossed it off to show that weβve completed it.
- πΊπΈUnited States Amber Himes Matz Portland, OR USA
I went through all unresolved threads and double-checked that they were resolved, and resolved them.
- Status changed to Fixed
6 months ago 11:28am 2 March 2024 - π¬π§United Kingdom alexpott πͺπΊπ
Committed and pushed b9dfd470e3 to 11.x and c57ce025c3 to 10.3.x. Thanks!
-
alexpott β
committed c57ce025 on 10.3.x
Issue #3420993 by Amber Himes Matz, andypost, sakthi_dev, smustgrave,...
-
alexpott β
committed c57ce025 on 10.3.x
-
alexpott β
committed b9dfd470 on 11.x
Issue #3420993 by Amber Himes Matz, andypost, sakthi_dev, smustgrave,...
-
alexpott β
committed b9dfd470 on 11.x
Automatically closed - issue fixed for 2 weeks with no activity.