Reviewing this.
The changes look good to me. But pipeline for phpunit is failing, thus moving to Needs work.
Hi @lisotton,
I reviewed the MR and it lgtm.
Hence, moving it to RTBC.
Updated the issue summary as suggested.
@norman.lol @jan Did the sugested changes to support relative URLs. Please review.
Applied the patch to resolve the inifinite recursion. Needs review.
Implemented the aria-labelledby approach to simplify submenu disclosure button label.
The issue needs review.
Generated MR for the patch #2. Needs review.
Checked the URL and replaced with # if not the valid URL. The issue needs review.
Hi @maskedjellybean,
I tried replicating the issue but the issue could not be replicated.
I am attaching screenshot for reference.
Please provide any steps to reproduce the issue.
Removed -settings from Search settings page path. The issue needs review.
Previous Path:
Altered Path:
Applied patch. The issue needs review.
Changed default loading to eager. The issue needs review.
The update_hook has been implemented and schema has been updated. The issue needs review.
Hi @rhovland,
You are saying that the feature is done but when I selected stars option in view display and selected shorthand link option.
It is not showing any stars, just a link of all reviews with the reviews count.
So can you please clarify what has been accomplished and what is needed to be done here?
Modified description for og:image field as per the suggestions. This needs to be reviewed.
Before:
Updated description:
Applied patch to ensure compatibility of module with Drupal 10.4 and above. It needs to be reviewed.
Implemented CategorizingPluginManagerInterface in StoryPluginManager to use ::getSortedDefinitions() in ::getComponentStories(). It needs to be reviewed.
Hi liam,
I am glad the issue is fixed. Please provide the due credit so that it motivates further to contribute in the community.
Thanks.
Applied patch to make the ajax callbacks use dynamic classes. It needs review.
Removing the hover text improves UX by reducing cognitive load and ensuring consistency with Drupal Core. Since the time increment is clear from the input itself, the tooltip is unnecessary. This also enhances accessibility by allowing screen readers to present information more effectively, creating a cleaner and more intuitive experience.
Removed the hover text from time element in widget. Kindly review.
Modified the changes as adviced by @esha_kundu. The changes need review.
Hi @anybody,
I noticed that a new MR was generated for this issue which includes the changes I did in MR 33 previously. While I appreciate the progress that the issue has been fixed, I’d like to kindly request due credit for my contribution to this issue as it motivates me to continue contributing to Drupal.
Looking forward to your response!
chhavi.sharma → changed the visibility of the branch 3492335-coding-standard-issues to hidden.
Made phpcs pipeline green. Please review.
Updated the depricated version.
I tried to solve the failing phpunit test but couldn't resolve it. Can anyone suggest something to pass that test?
Working on it.
Reviewed the MR.
The dependency is added successfully in composer.json.
The module functionality remains unaffected.
So, moving it to RTBC.
The phpcs pipeline is green.
So moving it to RTBC.
chhavi.sharma → made their first commit to this issue’s fork.
I reviewed the MR.
The cspell error is resolved and the pipeline is green.
Also, the merge conflict has been resolved.
So moving it to RTBC.
Working on it.
@pravesh_poonia,
I tried replicating the issue but the footer was already styled.
I also applied the patch but there were no changes in the theme UI.
I analyzed the changes the patch was intented to do. Therefore, I did those changes and created an MR for the same.
The changes need review.
Working on it.
Added all missing doc comments. Needs review.
Hi@chrisfromredfin,
I noticed that my contribution to this issue was merged, but I wasn’t credited. Could you please update it to reflect my contribution as it encourages continued participation and helps build a sense of collaboration in the community.
@quietone, Thanks for the heads up regarding assigning a core issue. I'll keep that in mind.
Also, I have pushed some changes that I have made till now. I am continuously working on this.
Working on it.
Made search input box visible.
Working on it.
Changed the label of the Night colour scheme to append “(low contrast)”.
Reviewed the issue but the issue was not replicated using the steps.
Checked it on Drupal version 10.3 and module version 1.10.
Attaching screenshot for reference:
Working on it.
Styled the Sort By component and passed the pipeline.
Working on it.
@quietone The patch was intented for 10.1.x branch but in the 11.x branch, the test file is only 70 lines long and all the testing functions which were available in the 10.1.x branch are missing from 11.x branch. So the patch changes can not be applied to the test file directly. So should I add all the missing functions and lines of code to the test file which are not included in the 11.x branch?
@quietone I tried to add test file changes but the UpdateSemvertestBase.php
contains 70 lines only but the suggested changes in the patch are at line 158 so I am not sure of the code in between those lines. That is why I didn't applied those changes manually.
Applied the changes of patch manually and created an MR against 11.x branch.
Working on it.
Solved the phpstan error and fixed the pipeline.
Working on it.
Can you please specify which tests are failing? According to MR, only the phpcs pipeline is failing. Do we need to clear that only or is there something else?
Hi @anybody,
Fixed all the phpcs issues and made phpcs pass. But still there is phpunit failing. I have no prior experience of writing tests. Therefore, unassigning myself from the issue.
Working on the code style and trying to make pipeline green.
Attaching the screenshot for reference.
Modifed the status message as per suggestion.
Working on it.
Hi @avpaderno @sujan-shrestha,
Resolved all the remaining phpcs errors.
Working on the remaining phpcs errors.
Hi @natts, I tried installing the 3.1.0-rc4 version of this module. I tried to replicate the issue by following these steps:
1. I added google service in the link icon service.
2. Then I added a link field in the article content type.
3. In the manage display section of "article", I selected "Link with service icon" in the format dropdown of link field.
4. I selected "none" in the "text alongside the icon" option.
5. I created a dummy article.
6. When I tried to click the icon, it didn't redirected me to google.com.
I also tried it on the tag "3.1.0-rc4", the issue persists there as well but it is functioning fine in 3.1.x branch.
I have attached the screen recording for reference.
I started working on it so if you permit me, I can reassign the issue to me and then build a patch for the 3.1.0-rc4 tag.
Working on it.