🇦🇷Argentina @tguerineau

Hi,

I've been working on the issue described here and have made progress in implementing the feature. Below is a brief overview of the changes and updates made:

1. Implementation Details: whereNull method added in the src/Collection.php file.

2. Manual testing that I made (using Devel module):

• Conducted tests with collections of nodes, ensuring that the method works as expected, particularly for fields within nodes.
• Verified the functionality with non-entity items to ensure broad applicability.
• Tests included various scenarios, including fields with simple values and more complex structures like paragraphs.
• All tests indicate that the method behaves correctly.

3. Automated Tests: Unfortunately, I am currently facing issues with my test environment, which has prevented me from creating automated tests for these changes.

4. Merge Request: I have created a Merge Request (MR) with the code for the implemented method.

Thank you for considering these updates, and I look forward to your feedback.

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

Hi,

I've been working on the issue described here and have made progress in implementing the feature. Below is a brief overview of the changes and updates made:

1. Implementation Details: whereNotInStrict method added in the src/Collection.php file.

2. Manual testing that I made (using Devel module):

• Conducted tests with collections of nodes, ensuring that the method works as expected, particularly for fields within nodes.
• Verified the functionality with non-entity items to ensure broad applicability.
• Tests included various scenarios, including fields with simple values and more complex structures like paragraphs.
• All tests indicate that the method behaves correctly.

3. Automated Tests: Unfortunately, I am currently facing issues with my test environment, which has prevented me from creating automated tests for these changes.

4. Merge Request: I have created a Merge Request (MR) with the code for the implemented method.

Thank you for considering these updates, and I look forward to your feedback.

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

Hi,

I've been working on the issue described here and have made progress in implementing the feature. Below is a brief overview of the changes and updates made:

1. Implementation Details: whereNotIn method added in the src/Collection.php file.

2. Manual testing that I made (using Devel module):

• Conducted tests with collections of nodes, ensuring that the method works as expected, particularly for fields within nodes.
• Verified the functionality with non-entity items to ensure broad applicability.
• Tests included various scenarios, including fields with simple values and more complex structures like paragraphs.
• All tests indicate that the method behaves correctly.

3. Automated Tests: Unfortunately, I am currently facing issues with my test environment, which has prevented me from creating automated tests for these changes.

4. Merge Request: I have created a Merge Request (MR) with the code for the implemented method.

Thank you for considering these updates, and I look forward to your feedback.

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

Hi,

I've been working on the issue described here and have made progress in implementing the feature. Below is a brief overview of the changes and updates made:

1. Implementation Details: whereNotBetween method added in the src/Collection.php file.

2. Manual testing that I made (using Devel module):

• Conducted tests with collections of nodes, ensuring that the method works as expected, particularly for fields within nodes.
• Verified the functionality with non-entity items to ensure broad applicability.
• Tests included various scenarios, including fields with simple values and more complex structures like paragraphs.
• All tests indicate that the method behaves correctly.

3. Automated Tests: Unfortunately, I am currently facing issues with my test environment, which has prevented me from creating automated tests for these changes.

4. Merge Request: I have created a Merge Request (MR) with the code for the implemented method.

Thank you for considering these updates, and I look forward to your feedback.

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

Hi,

I've been working on the issue described here and have made progress in implementing the feature. Below is a brief overview of the changes and updates made:

1. Implementation Details: whereInstanceOf method added in the src/Collection.php file.

2. Manual testing that I made (using Devel module):

• Conducted tests with collections of nodes, ensuring that the method works as expected, particularly for fields within nodes.
• Verified the functionality with non-entity items to ensure broad applicability.
• Tests included various scenarios, including fields with simple values and more complex structures like paragraphs.
• All tests indicate that the method behaves correctly.

3. Automated Tests: Unfortunately, I am currently facing issues with my test environment, which has prevented me from creating automated tests for these changes.

4. Merge Request: I have created a Merge Request (MR) with the code for the implemented method.

Thank you for considering these updates, and I look forward to your feedback.

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

Hi,

I've been working on the issue described here and have made progress in implementing the feature. Below is a brief overview of the changes and updates made:

1. Implementation Details: whereInStrict method added in the src/Collection.php file.

2. Manual testing that I made (using Devel module):

• Conducted tests with collections of nodes, ensuring that the method works as expected, particularly for fields within nodes.
• Verified the functionality with non-entity items to ensure broad applicability.
• Tests included various scenarios, including fields with simple values and more complex structures like paragraphs.
• All tests indicate that the method behaves correctly.

3. Automated Tests: Unfortunately, I am currently facing issues with my test environment, which has prevented me from creating automated tests for these changes.

4. Merge Request: I have created a Merge Request (MR) with the code for the implemented method.

Thank you for considering these updates, and I look forward to your feedback.

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

Hi,

I've been working on the issue described here and have made progress in implementing the feature. Below is a brief overview of the changes and updates made:

1. Implementation Details: whereIn method added in the src/Collection.php file.

2. Manual testing that I made (using Devel module):

• Conducted tests with collections of nodes, ensuring that the method works as expected, particularly for fields within nodes.
• Verified the functionality with non-entity items to ensure broad applicability.
• Tests included various scenarios, including fields with simple values and more complex structures like paragraphs.
• All tests indicate that the method behaves correctly.

3. Automated Tests: Unfortunately, I am currently facing issues with my test environment, which has prevented me from creating automated tests for these changes.

4. Merge Request: I have created a Merge Request (MR) with the code for the implemented method.

Thank you for considering these updates, and I look forward to your feedback.

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

Hi,

I've been working on the issue described here and have made progress in implementing the feature. Below is a brief overview of the changes and updates made:

1. Implementation Details: whereBetween method added in the src/Collection.php file.

2. Manual testing that I made (using Devel module):

• Conducted tests with collections of nodes, ensuring that the method works as expected, particularly for fields within nodes.
• Verified the functionality with non-entity items to ensure broad applicability.
• Tests included various scenarios, including fields with simple values and more complex structures like paragraphs.
• All tests indicate that the method behaves correctly.

3. Automated Tests: Unfortunately, I am currently facing issues with my test environment, which has prevented me from creating automated tests for these changes.

4. Merge Request: I have created a Merge Request (MR) with the code for the implemented method.

Thank you for considering these updates, and I look forward to your feedback.

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

Hi,

I've been working on the issue described here and have made progress in implementing the feature. Below is a brief overview of the changes and updates made:

1. Implementation Details: whereStrict method added in the src/Collection.php file.

2. Manual testing that I made (using Devel module):

• Conducted tests with collections of nodes, ensuring that the method works as expected, particularly for fields within nodes.
• Verified the functionality with non-entity items to ensure broad applicability.
• Tests included various scenarios, including fields with simple values and more complex structures like paragraphs.
• All tests indicate that the method behaves correctly.

3. Automated Tests: Unfortunately, I am currently facing issues with my test environment, which has prevented me from creating automated tests for these changes.

4. Merge Request: I have created a Merge Request (MR) with the code for the implemented method.

Thank you for considering these updates, and I look forward to your feedback.

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

Hi,

I've been working on the issue described here and have made progress in implementing the feature. Below is a brief overview of the changes and updates made:

1. Implementation Details: where method added in the src/Collection.php file.

2. Manual testing that I made (using Devel module):

• Conducted tests with collections of nodes, ensuring that the method works as expected, particularly for fields within nodes.
• Verified the functionality with non-entity items to ensure broad applicability.
• Tests included various scenarios, including fields with simple values and more complex structures like paragraphs.
• All tests indicate that the method behaves correctly.

3. Automated Tests: Unfortunately, I am currently facing issues with my test environment, which has prevented me from creating automated tests for these changes.

4. Merge Request: I have created a Merge Request (MR) with the code for the implemented method.

Thank you for considering these updates, and I look forward to your feedback.

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

Hi,

I've been working on the issue described here and have made progress in implementing the feature. Below is a brief overview of the changes and updates made:

1. Implementation Details: firstWhere method added in the src/Collection.php file.

2. Manual testing that I made (using Devel module):

• Conducted tests with collections of nodes, ensuring that the method works as expected, particularly for fields within nodes.
• Verified the functionality with non-entity items to ensure broad applicability.
• Tests included various scenarios, including fields with simple values and more complex structures like paragraphs.
• All tests indicate that the method behaves correctly.

3. Automated Tests: Unfortunately, I am currently facing issues with my test environment, which has prevented me from creating automated tests for these changes.

4. Merge Request: I have created a Merge Request (MR) with the code for the implemented method.

Thank you for considering these updates, and I look forward to your feedback.

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

Hi,

I wanted to provide an update on the Merge Request. Based on further review and considerations regarding performance, I have updated the Merge Request with a new approach that enhances performance.

Here's a summary of the revised implementation:
1. Revised Approach: The previous implementation involved enhancing the `filter` method and adding a `processEntityFields` method to handle both Drupal entities and non-entity items. However, this approach, while functional, had potential performance implications, especially when dealing with large collections or complex entities.
2. New Implementation: The new whereNotNull method has been streamlined to directly check the existence and non-nullity of the specified field in the collection items.

I have tested the changes proposed in Merge Request #357 and can confirm that they successfully resolve the styling issues.

The patch in MR#357 provides a viable solution to the issue.

Based on these findings, I recommend that the status of this issue be updated to "Reviewed & Tested by the Community".

Hi,

I'm providing an updated patch. This update is necessary due to the evolution of the 2.0.x branch since the last patch submission.

Context and Rationale:

The previous patch address-token_for_administrative_area-3112487-25.patch and the associated Merge Request were based on an older state of the 2.0.x branch. As a result, they could not be applied to the current branch due to significant changes that have occurred since then.
To address this, I manually applied the changes from the old patch to the latest version of the 2.0.x branch and created a new patch reflecting these updates.

Manual Testing:

• Conducted thorough testing of the new administrative_area_name token. This included:
  Integrating the token [node:field_address:administrative_area_name] into the default meta tags for content descriptions.
• Verifying token functionality on a node page with an address field.
• Confirming the token's presence and correct description in the token list at /admin/help/token.

I have created a patch for the 2.0.x.

I have adapted this patch for the 2.0.x branch, which includes support for an additional third address line.

Changes Made:
1. Renamed the token from 'address_lines_both' to 'address_lines_combined' to better reflect its functionality, as it now combines up to three address lines.
2. Updated the logic in address.tokens.inc to handle the third address line. The token now correctly concatenates the available address lines, separating them with commas as appropriate.
3. Manually tested the new token with the Meta Tag module. I used the [node:field_address:address_lines_combined] token in the default meta description for content. The token correctly outputs:

• Just the first line when only the first address line is present.
• The first and second lines, separated by a comma, when both are present.
• All three lines, each separated by a comma, when all are present.
• No additional text in the meta description when the address field is empty.

I have completed testing the '1 Week' option in the 8.x-2.x branch of the Google Calendar module, and I'm pleased to report that it works as expected.

My testing involved integrating a Google Calendar with the module and creating a variety of test events, some within the past week and some older. After selecting the new '1 Week' option in the "Past horizon" field, I observed that the module correctly imported events from the past week, while excluding older events.

Hello,

I am posting an updated version of the patch preferred_countries-2626982-53.patch → . This new patch addresses the issues encountered with the previous submission (preferred_countries-2626982-52.patch).

Please find the attached re-rolled patch for review and testing. I welcome any feedback or further testing.

Hi,

I've tested the preferred_countries-2626982-52.patch patch with the current 2.0.x branch and found that the patch doesn't apply due to code expected from the 8.x-1.x branch.

After applying the rejected hunks manually and resolving duplications in AddressDefaultWidget.php (defaultSettings, settingsForm and settingsSummary methods), I've successfully tested the module with preferred countries field.

The attached patch includes these modifications and works as expected. Please find my re-rolled patch attached for review.

Reworked #52 to apply to 2.0.x.

I have tested the patch 3411071-2.patch in drupal 10 and am providing feedback.

Testing Process and Observations:

1. Functionality: After applying the patch, I created and edited content with address fields to assess any impacts from the removal of #used_fields. The module behaved as expected, with no issues in creating, displaying, or editing addresses.
2. Cross-Browser Testing: The module functioned correctly across different browsers.
3. Role-Based Testing: Different user roles interacted with the address fields without any issues.
4. Error Checks: No related errors were observed in Drupal logs or browser consoles.

The patch, which focuses on removing deprecated code (#used_fields), appears to be functioning correctly without any adverse effects on the module's operation. Based on my tests, I believe the patch is successful in its intended purpose and does not introduce new issues.

Hi,

I have conducted testing, in drupal 10, on the patch submitted: allow-altering-the-redirect-url-success-3410776-3.patch. Here are the details of my testing process and findings:

Setup and Configuration:

• Applied the provided patch to the multi_domain_login module.
• Configured the module settings.
• Tested with various user roles, including Editor and Administrator.

The patch works as intended, successfully allowing the alteration of the redirect URL based on user roles. The implementation aligns well with Drupal's hook system and provides the desired flexibility for redirecting users post-login, based on their roles.

Hi,

I've been investigating the issue on a new Drupal 10.1.2 environment with the `ctools` module version 4.0.4.

During my testing no errors were encountered. I also checked the Drupal logs and did not find any related error messages.

I would like to request more detailed steps to reproduce this issue. The current steps provided mention using Drupal core version 10.1.2 and `ctools` versions 4.0.2 & 4.0.4, but further information on specific configurations or actions that trigger the error would be greatly helpful.

I'd like to bring to attention a related issue I recently worked on, which had a similar context regarding the deprecation of NodeType. In that issue, a patch was successfully applied and tested, resolving the deprecation warning without causing any new problems. The patch involved updating references from NodeType::class to EntityBundle::class in the ctools.module file.

Given the similarity in the nature of the deprecations and the components involved, I am wondering if the patch from the previous issue might also be relevant here, or if the issues could be connected in some way. The previous issue can be found at Deprecated class Drupal\ctools\Plugin\Condition\NodeType 🐛 Deprecated class Drupal\ctools\Plugin\Condition\NodeType RTBC .

Hi,

I've successfully tested the patch provided in comment #4 on the ctools module version 4.0.4. Here are the details of my testing procedure and findings:

Environment:

- Drupal Version: 9.5
- Module version: 4.0.4

Patch Application:

- I downloaded the patch named "3403902-ctools-deprecated_nodetype_code_fix-2.patch" in the #4 comment and applied it to my local Drupal installation using the `patch` command.
- The patch was applied without any issues.

Testing Procedure:

- After applying the patch, I cleared the Drupal cache.
- I navigated to the Upgrade Status report page and ran the report.
- The previously reported warning regarding the deprecated `Drupal\ctools\Plugin\Condition\NodeType` class was no longer present.

Observations:

- Post-patch, all functionalities of the ctools module appeared to operate normally.
- No new warnings or errors were observed in the system logs.

Based on my testing, the patch successfully resolves the issue.

Hello,

I am looking to contribute to this issue and have set up a Drupal 10 environment with the CTools module version 4.0.4 as mentioned. However, I am unable to find the auto-submit.js file and the ctools_views.libraries.yml file within the module's directory. The once function also does not seem to be used anywhere in the module's current version.

Could you please confirm if the issue is indeed present in version 4.0.4 of the CTools module? If it's a different version that is affected, could you please specify so I can attempt to reproduce the issue accurately?

Thank you for any clarification you can provide.

Hi @solideogloria,

I wanted to share my progress on creating tests for the IP Consumer Auth module. I've been working on developing a comprehensive testing approach to cover various scenarios and ensure robust module functionality. More scenarios still need to be added.

However, due to time constraints and other work commitments, I'm unable to continue this work for the time being. I'm pushing my current test code, which includes an initial implementation of functional tests for the module. The tests are designed to validate the behavior of IP whitelisting and blacklisting, and how they affect access to endpoints for different user roles.

Please note that I have not been able to fully verify these tests due to environmental issues with my Lando setup, which prevented successful execution of PHPUnit tests. The tests are therefore unconfirmed in their current state but should serve as a starting point for further development and refinement.

I hope this contribution will be useful and I look forward to seeing how the tests evolve. I regret that I'm unable to continue at this time, but I am keen to assist further when possible.

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

Hi @solideogloria,

Yes, I have some experience writing tests.

I can work on adding the tests to cover all possible cases, to ensure that the module functions as it should.

I have conducted further testing of the ip_consumer_auth module with the applied patch in a Drupal 9 environment. Here are my updated findings:

• Whitelist Test: When IP 172.22.0.1 was whitelisted, anonymous users received a 200 OK status with no output, which is expected behavior indicating access is allowed.
• Blacklist Test: Using the same IP as a blacklist entry or a fake IP 111.2.3.1 in the whitelist resulted in the error {"message":"The used authentication method is not allowed on this route."} for both anonymous and authenticated users. This was unexpected, as the response suggests an authentication method issue rather than an IP restriction response.
• Authenticated Users: Consistently received the error message regardless of the IP configuration, indicating a potential misconfiguration in authentication methods for the REST endpoint.

Behavior without the Module:

• When the ip_consumer_auth module was disabled, accessing the endpoint did not yield any authentication errors, suggesting that the REST endpoint itself is configured correctly to allow access without additional authentication methods.

Conclusion and Queries:

• The behavior suggests that while the whitelist functionality of the ip_consumer_auth module works correctly for anonymous users, there are issues when IPs are blacklisted or not included in an active whitelist. Specifically, the consistent authentication method error is perplexing, given that it does not occur when the module is disabled.
• Are there specific configurations within the ip_consumer_auth module that might be causing this behavior, or could this indicate a deeper issue with the patch or the module's interaction with Drupal's authentication system?

Hi @kenyoOwen,

Thank you for the correction @thakurnishant_06.

If anything else needs to be corrected, let me know.

Hi,

I've been working on the issue.
Here's a summary of the approach and the progress made:

Implementation Details:

• Enhanced the filter method in Collection to differentiate between Drupal entities and non-entity items.
• Added a processEntityFields method to simplify entity fields for consistent handling by the filter method's callback.
• The whereNotNull method utilizes this updated filter method to check for non-null values in both entities and non-entity items, making it versatile across various types of collections.

Manual testing that I made (using Devel module):

• Conducted tests with collections of nodes, ensuring that whereNotNull works as expected, particularly for fields within nodes.
• Verified the functionality with non-entity items to ensure broad applicability.
• Tests included various scenarios, including fields with simple values and more complex structures like paragraphs.
• All tests indicate that the method behaves correctly, filtering items based on the presence of non-null field values.

I am in the process of designing and implementing tests that will be added to ensure the correct operation of the whereNotNull method with collections of paragraphs or nodes, as per the issue description.

I look forward to your feedback.

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

Hi,

I've updated the README.md file to align with the format specified in the Drupal README.md template. The Table of Contents now follows the plain text format without hyperlinks.

This update has been committed and pushed, and the issue status is set to "Needs review.".

Additionally, I wanted to briefly mention that implementing a clickable Table of Contents could enhance user navigation for longer documents. This is a common feature in many open-source projects. I understand the importance of following Drupal's standards, but I thought it worthwhile to suggest this potential improvement for future consideration.

Hi @kenyoOwen,

Thank you for your feedback. I have made the change to the README.md as suggested.

The changes have been committed and pushed. The issue status has been updated to "Needs review" for the latest revision to be checked.

Hi,

While examining the issue, I observed that the content text is both larger and darker than the help text, which seems to provide clear visual distinction.
Attached is a screenshot showing this.

But if the goal is to enhance this distinction further:

The current styling for the help text is:

ul.tips, .form-wrapper .description, .form-item .description {
    opacity: 0.8;
    /* other styles */
}

Considering accessibility guidelines and maintaining adequate color contrast, I propose reducing the opacity to 0.7. This change is subtle but could provide a clearer differentiation without compromising accessibility.

I have pushed a commit with this change for review.

Thanks!

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

Hi @thakurnishant_06,

I have updated the README.md file following the Drupal README.md template provided in the documentation guidelines.

The sections for Troubleshooting and FAQ were not included, as they are optional and there's currently no information to add for those categories.

The updated README is now pushed to the repository. I have changed the issue status to "Needs review" for the latest updates to be examined.

Hi @kenyoOwen,

I wanted to provide an update on the progress of this issue. I have reviewed the latest commit by @thakurnishant_06, and it appears that they have addressed the feedback provided. The commit includes the addition of the "Project name and introduction" section along with a clear "Installation" section as requested.

The issue status was not updated to "Needs review" after this commit, so I have gone ahead and updated it to reflect the current state of the issue for review.

I believe the issue is now ready for the next steps in the review process. Please let me know if any modifications are required.

I've addressed the issue with the following changes:

• A new class called 'comment-new-indicator' has been added to the bellamy.json configuration file to target the <mark> element within article.new.
• Padding has been applied to the 'comment-new-indicator' class in the bellamy.css file to ensure adequate spacing around the 'new' text for comments.

These changes have been tested locally and verified to add the desired padding without affecting other elements or comment styling.

I have pushed the commit and opened a merge request.

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

Hi @baldwinlouie,

I have completed the rebase of the branch once again as requested. During the process, I encountered and resolved numerous conflicts to ensure that the latest changes from the 6.x branch are incorporated with the fixes intended for this issue.

Given the extensive nature of these conflicts, I would appreciate a thorough review to confirm that all intended functionality is intact and that no contributions have been inadvertently omitted.

The branch is now up-to-date. Please let me know if there's anything else that needs to be addressed.

Hi @pdureau,

Following up on the recent discussion, I've created a new branch 3396957-Single-checkbox-radio-error with the updated fix where I've removed the ! operator as suggested. I've pushed this branch and created a new merge request to address the typo mentioned earlier.

Here's the link to the new merge request for review.

I look forward to any reviews or comments.

Hi @pdureau and @mh_nichts,

Thank you for the catch! I've reviewed the code and you're correct about the error class condition.

I've made the necessary update and pushed the commit.
I've observed that my latest commit addressing the error class condition is present in the repository on the original branch, which confirms the push was successful. However, this update isn't reflected on the changes page linked from the issue.

Could anyone clarify if I should create a new merge request for this fix, or if there is another step I should take?

Appreciate your help on the next steps.

I've addressed the issue with checkboxes erroneously losing their error classes upon interaction. Here's a brief overview of the fix:

• Implemented a check for the 'checked' property of checkbox inputs to accurately determine their state.
• Maintained the existing validation logic for other types of inputs.

This update ensures that the error class on a checkbox is only removed when the checkbox is actually checked, which aligns with the expected behavior when the 'Automatically remove error classes when values have been entered' setting is enabled.

The MR contains the necessary adjustments.

I created a MR for review and would appreciate any feedback.

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

Hi @pdureau, thank you for pointing that out. I overlooked the distinction between title as a text property and label as a more complex renderable slot. Please let me know the findings of your deeper look, and I'll be ready to make necessary adjustments. Appreciate the thorough review!

Hi @prudloff,

I've been working on implementing a solution for deferring scripts effectively using the suggested Html::load() and Html::serialize() methods. The initial approach we discussed seemed promising, but during implementation and testing, we've encountered some significant challenges that I'd like to outline:

1. Document Structure Changes: Utilizing Html::load() for the entire document inadvertently modifies the structure. Specifically, elements that are originally in the <head> tag are being moved into the <body> tag. This is problematic as it alters the expected layout and behavior of the page, leading to visual discrepancies and potential functional issues.
2. DOM Manipulation Side Effects: In an attempt to circumvent the above issue, I tried various methods to isolate the manipulation to the <head> content. This included wrapping content in temporary containers and placeholders to maintain structure. Unfortunately, these methods introduced additional complexity without resolving the core issue - that Html::load() and Html::serialize() are not preserving the placement of head elements when serializing the document back to HTML.
3. First Approach Functionality: It is worth mentioning that the initial non-DOMDocument approach we developed does not encounter these structure-related issues and works as expected. It effectively transforms the <link> elements for CSS without disrupting the document structure.

Given the problems faced with the current Html::load() and Html::serialize() strategy, I propose revisiting the initial approach for further consideration. It offers a stable solution with the desired functionality without compromising the document's integrity.

I am open to feedback and any suggestions on alternative methods.

Hello,

I've been working on addressing this issue and have created a MR with the necessary adjustments. Here's an overview of the changes:

1- Removed Exclusions: In the bootstrap_barrio.theme file, I've removed the lines that excluded select and textarea from floating labels support.

2- Textarea Adjustments:
- Twig template updated: I modified the textarea twig template to adjust its height based on Bootstrap's recommendation. This will ensure a more consistent and visually appealing form design.
- Height Consistency: I've updated the textarea twig template to include a height of 150px. This ensures that textareas maintain a consistent height irrespective of whether the floating label feature is activated or not.
- Hook Preprocess for Textarea: I introduced a hook_preprocess_textarea in the theme file. This hook checks if the 'float_label' theme setting is enabled and accordingly sets a placeholder for the textarea. This placeholder is essentially the title of the textarea, making it intuitive for end-users.

3 - Floating Labels for Select Inputs: Following the Bootstrap documentation, the <label> for .form-selects is always displayed in its floated state. This caters to Bootstrap's specifications and ensures a unified design approach.

Please review the changes and let me know if there are further adjustments needed.

Thank you for your feedback!

My apologies for the oversight. I've now created the MR. You can review and merge it. Appreciate your patience!

Thank you for your feedback!

My apologies for the oversight. I've now created the MR. You can review and merge it. Appreciate your patience!

Certainly!

I've created a Merge Request (MR) with the changes.

Let me know if there's anything else needed. Thank you!

Certainly!

I've created a Merge Request (MR) with the changes.

Let me know if there's anything else needed. Thank you!

I realized I missed addressing the button pattern in the initial patch. This updated patch now encompasses changes for both the link and button patterns as originally mentioned in the issue.

In this update:

- Added adjustments to pattern-button.html.twig using {% set title = title|default(label) %}.
- Retained the link pattern modifications from the previous patch.

Kindly review, and I apologize for the oversight in the initial patch. Thank you!

Hi,

I've addressed the issue where external links with a target="_blank" attribute that didn't have a provided title were rendering the title attribute as " - new window". This was identified as a potential accessibility concern.

Changes Made:

1- Modified the pattern-link.html.twig template to check if a title is set for the link.
2- If the title isn't set, the visible link text (label) is used as the title.

I've attached a patch which contains these changes.

Please review the attached patch and let me know if there are any concerns or additional changes needed.

Hi,

I've looked into the issue regarding the single checkbox/radio error class/message and have identified the root cause.
I've attached a patch that implements this fix. Here's what I've changed:

1- Replaced the condition checking for #delta with #error_no_message to determine if an error should be added.
2- Adjusted the logic for setting the $error_class variable based on this new condition.

Please test the patch. Looking forward to feedback!

Hi,

I've looked into the issue described and have made the following changes:

1- Updated prototype_prepare_menu_items Function: Modified the function to add a data-plugin-id attribute for each menu item based on the menu link's plugin ID.

2- JavaScript Updates: Changed the way IDs are retrieved in the JavaScript. Instead of transforming the text of the menu item, we now read the data-plugin-id attribute.

3- Typo Fix: Corrected the typo in the JavaScript where "pannel" was misspelled. It's now correctly spelled as "panel".

I've attached the patch that includes these changes. I'd appreciate any feedback or testing on this. Please let me know if there are additional adjustments needed!

Hi, I have worked on the issue.

Proposed Fix:

Removed the max-height: 458px; style from the #wrapper img selector. This change ensures that images retain their natural aspect ratio and aren't artificially constrained by height, which was causing the stretching issue.

Testing:
I've tested the change across various views and content types within the theme. All images appear to retain their natural aspect ratios, and the stretching issue isn't observed anymore. However, given the global nature for the style (being in page.css), I recommend further testing.

Patch:

I've attached a patch that implements this change. Please review and test it.

Hi @JFKiwad ,

I've been investigating the issue mentioned regarding the proxy.js file being tracked by Git and discovered that the actual path of the existing proxy.js file is src/kits/radix_starterkit/config/proxy.js, and there is a proxy.example.js file within the same directory.

With this understanding, I've updated the .gitignore to correctly exclude src/kits/radix_starterkit/config/proxy.js. I've committed this change.

However, I'd like to seek clarification here: can anyone confirm if the src/kits/radix_starterkit/config/proxy.js is indeed the file this issue is referring to? I made an assumption based on the file's location, and I want to ensure it's correct. If this issue pertains to a different proxy.js file, could you please provide its precise location?

Awaiting your confirmation to proceed correctly with this issue.

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

Hi, I have worked on the issue of replacing the README.txt file with a README.md file to take advantage of Markdown's formatting capabilities, improving the readability and visual appeal of the project documentation.

Here are the steps I took to resolve this issue:

- Renamed the existing README.txt to README.md.
- Converted the plain text formatting to Markdown syntax for enhanced rendering on GitLab.

The changes have been pushed to the issue fork.

Please review the MR and let me know if any further changes or adjustments are required.

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

Hi @baldwinlouie ,

I've updated the branch with the latest changes from the 6.x branch as requested.

- Successfully rebased our branch onto the origin/6.x branch.
- Resolved all conflicts (if any) during the rebase process.
- Pushed the updated branch to the remote repository.

The branch is now up-to-date and ready for a final review and testing. Everything seems to work fine, please let me know if there's anything else that needs to be addressed.

Hi @baldwinlouie , in order to close this issue I made a commit where I reverted the changes you mentioned.

I did a test and everything seems to work fine.

These changes have been committed and pushed to the issue fork for review and testing.

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

Hi @baldwinlouie, thank you for your review.

SCSS Compilation Fix:

I have made the suggested adjustment:

&:checked + label::after {
  transform: scale(1);
  animation: button-radio-ripple 0.5s linear;
}

This alteration aligns with the defined @keyframes button-radio-ripple.

The MR is now updated with this change.

Hi @baldwinlouie , thank you for your review.

Regarding the Provided Patch:
I’ve noticed the requirement for the compiled main.css. While preparing to compile, an unrelated issue was identified which prohibited successful compilation of the SCSS files:

Issue Identified:

In the _formelements.scss file, there’s a reference to a variable $button-radio-ripple that seems to be undefined or possibly mistyped:
animation: $button-radio-ripple 0.5s linear;

Given the existence of $button-radio-ripple-size, it seems plausible that this might be the intended variable, but we need collective insights to confirm this.

My Approach:

To proceed and provide a compiled main.css, I opted to adjust the variable reference to $button-radio-ripple-size. This enabled the compilation to proceed, however, this introduces a change that is not directly related to the initially addressed menu spacing issue.
- The MR Includes both the original fix for the menu spacing and the adjustment to enable SCSS compilation.

Seeking Guidance on Issue Management:

Given that these fixes are bundled, I wanted to seek your advice on the best approach to manage this within the issue queue:

Option 1: Keep both fixes within the current issue and merge request, with detailed notes explaining the inclusion of the SCSS compilation fix.
Option 2: Create a new issue for the SCSS compilation error and submit a separate merge request, even though the current MR includes both fixes.

I’ll proceed to update the MR with both fixes (the compiled main.css and the compilation error). Your feedback, testing and insights on both issues will be valuable. Thank you.

Upon reviewing the Merge Request (MR) and your proposed changes, I observed that the styles have been placed in main.css and specifically target .menu--custom-menu. While this does address the issue for custom menus, it seems like the problem also persists in other menu blocks across the theme, such as the Tools menu and Footer menu, indicating a more global styling adjustment might be beneficial.

1. Spacing Issue:
- Affected Areas: Tools menu, custom menus, and footer menu exhibit identical spacing issues wherein the menu items are conjoined without adequate spacing.
- Visual Impact: This absence of spacing impacts visual separation and readability of menu items.
2. Coloring Issue:
- The previously reported color issue (black text on a black background) seems to be non-replicable in the current theme version. However, it is crucial to ensure that any introduced styles do not inadvertently reintroduce this issue.

Proposed Solution:

In light of the above, a global styling adjustment is proposed that ensures consistent spacing across all menu items within the theme, excluding the last item to prevent undesired end spacing:

/* Global styling for all menu blocks */
nav {
  &.block-menu.navigation li {
    &:not(:last-child) {
      margin-right: 10px;
    }
  }
}

I have created a new branch and applied these changes.

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

Thank @SandeepSingh199 for taking the time to review the patch and providing valuable feedback!

I have updated the issue state to "Needs Review" as per your suggestion.

@pdureau Thank you for your detailed feedback and guidance. I've made adjustments based on your comments and suggestions:

1. Pattern Setting for title_id: title_id has been exposed as a pattern setting within the pattern definition file, allowing developers to understand and utilize it effectively.

2. Usage of default() filter with random() function: The default() filter has been applied to title_id, adhering to the best practices outlined in issue #3380233.

3. Declaration and Usage of title_id: title_id is utilized in aria-labelledby and declared appropriately within the HTML structure to enhance accessibility.

These changes have been committed and pushed to the issue fork for review and testing.

I have created a patch to address the issue of replacing the aria-label attribute with aria-labelledby on the nav element of the side menu, as per the DSFR 1.8 changelog. The patch includes a unique ID for sidemenu__title which is then referenced by aria-labelledby in the nav element.

In my local environment, it appears that the block--system-menu-block.html.twig template from the core system module is being used instead of the pattern-side-menu.twig template from the ui_suite_dsfr theme. I have created the patch based on the assumption that the pattern-side-menu.twig template should be used.

I am not entirely sure if there's additional configuration needed to ensure the pattern-side-menu.twig template is used, so any feedback or testing from others would be greatly appreciated.

Attached is the patch for review and testing.

Thank you for taking the time to review this patch!

Patch Updates:

- Use switch instead of else if.
- Testing aria-describedby Attribute: testAriaDescribedbyAttribute(), is introduced to ascertain the correct setting of the aria-describedby attribute by the maxlength.js script.

Test Failures Noted:

We've observed the following test failures in the CI.
Unfortunately, I have been unable to run the tests locally, which has limited my ability to further diagnose and resolve the test failures.

Potential Causes:
Given the nature of the updates in the patch, it's plausible that the changes to the character count announcements and their respective behaviors might be influencing the test outcomes. Specifically, the additional announcements or altered text/behaviors might not align with the current test expectations.

Next Steps & Request for Assistance:

- If the test failures are related to the updated announcement behaviors, the tests might require updates to reflect the new expectations.
- Any advice, suggestions, or contributions to further diagnose or fix the test failures would be invaluable.

I came across the same issue in my local environment. Based on the initial patch and the subsequent feedback, I've updated the styles in the holy/sass/components/02-molecules/field/_field--type-comment.scss file.

The compiled CSS (in css/style.css) has also been updated accordingly to reflect these changes.

The modified styles should address the padding issue with the new tag.

Patch attached. Kindly review.

I've been working on the feedback provided and have made some updates to the module in a new branch. Here's a brief summary:

1. DOM Parsing over Regex: I've switched the logic from using regex patterns to a DOM-based approach. This helps in addressing the limitations of regex in parsing HTML and provides a more robust and flexible way of handling different <link> tags in the document.

2. Maintaining <link> attributes: The new changes ensure that attributes such as media are retained when the <link> tags are modified.

3. Selective Handling of External Links: The new logic respects the css_defer_external setting, ensuring that only the desired links (either internal, external, or both) are deferred based on the configuration.

I've pushed these updates to a new branch on the issue fork.

Testing: I've conducted preliminary tests, and the changes seem to work as expected. However, I'd appreciate it if others could also test and share their feedback.

I kindly request everyone to review the changes. I believe these changes address the concerns raised in this issue, but I'm open to further improvements.

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

In order to address the issue I made a commit.

Here's a brief overview of the changes:

1. Refactored Mermaid JS Integration: Instead of initializing Mermaid for each diagram element multiple times, the code has been optimized to ensure each .mermaid element is initialized only once. This change not only boosts performance but also prevents potential rendering anomalies.

2. Introduced jQuery's .once() Method: To achieve the above, I've incorporated the jQuery .once() method. This ensures that each .mermaid element is processed just a single time, avoiding redundant operations.

By implementing these changes, the Mermaid JS file now runs efficiently, ensuring diagrams are processed correctly.

Review the commit and provide feedback. If there are any concerns or further improvements to discuss, I'm open to suggestions.

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

I've attempted to reproduce the issue described. I have the same version of the Metatag module as mentioned in the issue. Here are my findings:

1. I set up default Metatag configurations for the "Article" content type with the following settings:

Title: weknow ' Inc ` "
Description: weknow " Inc ` '

2. After checking the frontend, the meta tags appear to be correctly rendering:

Description meta tag: content="weknow " Inc ` ' "
Title tag: <title>weknow ' Inc ` "</title>

Based on these observations, I wasn't able to replicate the issue. Everything appears to be functioning as expected in my test.

Could you please provide some additional details to help us better understand the problem?

- What version of Drupal are you using?
- Are there any other modules or custom configurations that might be interacting with the Metatag module in your setup?

I have reproduced the issue and confirmed that decimal types are not aligning to the right in tables. The problem seems to be with the line of code that specifies the types of fields that should be right-aligned.

I've tested the fix suggested in the issue description, changing 'decimal' to 'numeric', and it works as expected. Decimal fields are now aligning to the right.

Here's the patch that implements this change: datafield-fix-decimal-alignment-3385183-2.patch

Steps to Test:

1. Apply the patch.
2. Create or edit a content type to include a DataField with a numeric column.
3. Create new content and observe that the numeric values align to the right in the table view.

Please review and let me know if there are any issues.

Hi @rmorelli,

I've tried to reproduce this issue on a Drupal 10 environment with PHP 8.1 and the Charts module version 5.0.x-dev, which is the same version mentioned in this issue.

Here are the steps I followed:

1- I added a chart field to the Article content type with unlimited values.
2- Created a new article without any data in the chart field and saved it. I observed that no empty chart was added.
3- I edited the article but not the chart field and saved it again. Still, no empty chart was appended.
4- Finally, I added data to the chart field, saved the article and then I went to edit mode and saved it. I observed the chart displayed correctly with the added data.

Upon following these steps, I couldn't replicate the issue of empty charts being added or being unable to remove them.

Questions:

- Could you provide more details on your environment?
- Are there any other modules interacting with the Charts module?
- Any special configurations or settings you have applied?

Thank you!

I've been working on addressing the issue as described, and here's a summary of the changes I made:

1. I've updated the enableBootstrapAutomatically() function to return JSON instead of an integer. This change is consistent with the recent updates made to the hookModalSubmit() method as per issue #3380924.

2. Modified the Ajax code in modal-page.js to correctly handle and act upon the new JSON response format.

Attached is the patch with the changes I've made. Feedback and further testing are appreciated!

Issue Update: 3383327

I'd like to apologize for the oversight in my previous commit. I mistakenly pushed changes for this issue to a branch intended for a different issue within the same contrib theme.

Actions Taken:

I've pushed a new commit to revert the unintended changes.
The branch now only contains changes relevant to the "Comments color" issue.
I understand the importance of maintaining clarity in version control, especially in collaborative projects. Please review the updated changes and let me know if there are any concerns.

Thank you for your understanding.

In order to address the issue I made a commit.

Proposed Solution: I've addressed the table header color issue by increasing the CSS specificity for the affected styles. In order to ensure compatibility with various use cases and avoid potential regressions, I've retained the existing !important declarations for now. However, with the increased specificity, some of these might be redundant. It would be worthwhile to have a follow-up issue to assess and potentially clean up any unnecessary !important declarations.

Changes Made:

- Updated the CSS selector from table { to table.table { to increase specificity.
- Retained existing !important declarations for safety.

Testing: The changes have been tested locally in various scenarios, and everything seems to function as expected. I'd appreciate feedback.

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

Thank you for the feedback.

I made the following changes:

1. I have created an issue fork and opened a Merge Request for it. I apologize for the oversight – I mistakenly created three branches in the process. I've opened a Pull Request for the correct branch, and I'll ensure to be more careful in the future. Please let me know if there's any cleanup needed on my end regarding the extra branches.

2. I've made the necessary changes to the SCSS files as you pointed out, compiled them, and both the SCSS and main.css files have been updated in the Merge Request.

Let me know if there are any additional changes or adjustments needed.

Thanks again for your guidance and patience.

In order to address the issue I made a patch.

Patch Description:

This patch addresses the visual discrepancies between the default textarea and the CKEditor appearance when different text formats are selected. The proposed changes ensure a cohesive and harmonized visual experience across various text formats.

Changes Made:

1. Updated CKEditor background, text color, font size, and weight to match the textarea styles for consistency.
2. Adjusted the focus border color of CKEditor to align with other form elements.

Test made:

- Ensured the styles look appropriate across different viewports.
- Checked the visual appearance of various CKEditor buttons and functionalities for consistency.

Feedback:

Given that these are visual changes, I'd appreciate feedback on the aesthetics. Please review the updated CKEditor appearance and let me know if further refinements are required.

Patch Update

Summary:

This patch addresses the issue of making the countdown message screen reader compatible. The primary changes involve associating the countdown message with its corresponding field using the aria-describedby attribute and announcing important character count milestones using Drupal.announce().

Details:

1. Associated the countdown message with its input field using the aria-describedby attribute, improving accessibility for screen reader users.
2. Introduced logic to announce when there are only 10 characters left, when there are no characters left, and when the character limit is exceeded. These announcements are wrapped in Drupal.t() to ensure they're translatable, enhancing compatibility for multilingual sites.
3. The changes were tested using the Screen Reader extension for Chrome.

Looking forward for reviews and feedback.

In order to address the issue I made the following changes:

- Refactored the JavaScript code to resolve ESLint errors, Used eslint for Drupal JS coding standards → .
- Simplified the initialization of the ml variable within the local script scope.

Manual Testing:

- Checked the retention of the previous maxlength value set on a field, including summaries.
- Verified the saving of maxlength settings.
- Ensured that each maxlength limit method operates as expected.
- Confirmed that changing maxlength limit methods works correctly.
- Tested updating from the latest release to the newest version of the main branch and didn't encounter any issues.

Observations:

-I observed the code pattern const ml = ml || {}; which gives an impression that ml might be a global variable. However, after thoroughly checking the module's directory and the broader codebase, I didn't find any external declarations for ml.
-I've refactored the code to initialize ml only within the local scope of our script. Manual tests seem fine, but I'd appreciate if reviewers can validate this change and ensure there's no hidden dependency on a global ml.

I've been working on addressing the issue as described, and here's a summary of the changes I made:
- Updated ModalAjaxController::hookModalSubmit() to return a JSON response instead of integers: Instead of directly echoing TRUE or FALSE and exiting, I transitioned to using structured JSON responses. This approach provides more context on the outcome, whether it's a success or a failure, and offers additional information that can be helpful for debugging.

- Modified the Ajax code in modal-page.js to correctly handle and act upon the new JSON response format. Error messages are now logged to the console for debugging purposes.

Attached is the patch with the changes I've made. Feedback and further testing are appreciated!