I have refactored the country lookup logic into a Drupal service and addressed the requested improvements:
Converted the static ConregCountry class into a proper CountryService with a corresponding interface.
Implemented PHP 8 constructor property promotion in the service and the Registration form.
Enabled autowiring in conreg.services.yml for cleaner service registration.
Updated the Registration form to use the service via dependency injection and removed the obsolete ConregCountry class.
Verified that all existing kernel tests pass.
I have moved the SimpleConregAdminCheckIn to src/Form/Admin/CheckInMembers, updated the routing, and also added a test for the same.
Please review.
I noticed this change removes the focus outline entirely for the active tab. While this visually hides the white corner issue, it also removes the focus indicator altogether, which impacts keyboard navigation and accessibility.
The underlying issue seems to be the outline not respecting border-radius, which is why the corner artifacts appear. Instead of disabling focus styles, we should replace the outline with a custom focus style (e.g. using box-shadow) that matches the rounded tabs while keeping focus visible.
Also i was working on it as mentioned in the comment, Please make sure to follow drupal guidelines as you are an experienced contributor. And you have only pushed the changes but have not created a MR, so please create a MR after pushing the changes.
Thank you
Made the changes and now the changes are being reflected as expected for me
Please review.
I am able to reproduce this issue.
I am working on the solution.
Thanks for the clarification. The test failure makes sense now test_element_access intentionally denies access to some elements, so enforcing element access on the edit route correctly results in a 403 and the Save button not being present.
My earlier change allowed edit access to keep existing tests passing, but I agree this bypasses the intended behavior. It looks like the real fix needs either an update to WebformUiElementPropertiesTest to skip elements with denied access, or adjusting the test_element_access fixture expectations.
These changes are working for me correctly, Now the content and role based filtering are working.
Please review.
divyansh.gupta â created an issue.
Reuploaded the file with proper naming structure.
I was able to resolve this error for 4.1.0 release,
Please review the patch attached.
Hello @jrockowitz,
should i add a sub-module to this webform module, or create a separate contrib module for this
Please guide me for this.
I was able to reproduce this bug and the patch successfully applied and resolved the issue for me.
Please review
@ptmkenny,
No i rebased the mr on 2.0.x only, also there are no extra changes showing in Mr and pipelines is also all green,
Please review
Working on it !!
Made some changes.
Please review
Updated the code according to the comment.
Please review!!
Rebased the MR.
Please review
Working on the rebase
I updated the title callback to retain the page context and include the migration/group label.
Please review!!
Sorry @guignonv, but i am not able to use this module i am seeing this error again and again
Error message:
External entities requires the galbar/jsonpath library.Although i installed galbar/jsonpath mutliple times.
Ok @guignonv, no worries i will work on the remaining fixes.
Working on it
Added changes from the MR such that it does not disturb css of any other theme while include the polishing css from gin.
Please review
Rerolled it, Please review!!
divyansh.gupta â made their first commit to this issueâs fork.
Created a new MR targeting 6.3.x.
Please review MR 719
divyansh.gupta â changed the visibility of the branch 3369136-batch-download to hidden.
Working on the reroll
@johnv,
i have updated my MR to make minimal variable changes and i was able to reproduce the error but by these following steps:
- Set Number of slots per day to 3.
- Create a node with an Office Hours field (exceptions enabled).
- Add 2 exceptions and save.
Now edit the node again:
- Click âAdd exceptionâ 3 times â 3 new empty rows appear.
- Fill the 2nd exception with one or more timeslots.
- Click âAdd exceptionâ again â too many rows are added (2 instead of 1).
- Further clicks only add one row at a time.
I also tested your patch, but I still ran into the original behavior described in the issue. Itâs possible I may have missed a configuration detail on my side â just wanted to share my findings.
Can you also please review my latest changes.
Made some changes and now things are working perfectly fine on my local.
Please review.
Working on it!
I was able to reproduce the bug and had created a MR which resolved the bug for me,
Please review
Working on it!
Rerolled the MR,
Please review!!
Working on the rebase.
Made the changes and verified them,
Please review.
Working on it !!
Iâve updated the plugin annotation to cover all core text field types and widgets (string, string_long, text, text_long, text_with_summary and their widgets). Tested and confirmed the Automator now shows up for each of them.
Please review!!
yes @mably,
i think someone else would be able to write proper tests.
i am unable to reproduce the issue using this test even after trying multiple things,
My test file is:
<?php
namespace Drupal\Tests\domain_access\Functional;
use Drupal\Tests\domain\Functional\DomainTestBase;
use Drupal\domain_access\DomainAccessManagerInterface;
use Drupal\user\RoleInterface;
/**
* Reproduces cross-domain content creation issue.
*
* @group domain_access
*/
class DomainAccessCrossDomainCreationTest extends DomainTestBase {
/**
* {@inheritdoc}
*/
protected static $modules = [
'domain',
'domain_access',
'field',
'node',
];
/**
* {@inheritdoc}
*/
protected function setUp(): void {
parent::setUp();
// Clear default permissions for authenticated users to avoid bleed-through.
$this->config('user.role.' . RoleInterface::AUTHENTICATED_ID)->set('permissions', [])->save();
// Ensure required content type exists when not using the standard profile.
if ($this->profile !== 'standard') {
$this->drupalCreateContentType([
'type' => 'page',
'name' => 'Basic page',
'display_submitted' => FALSE,
]);
}
// Create exactly two domains for this test.
$this->domainCreateTestDomains(2);
}
/**
* A user assigned to domain B should not be able to create on domain A.
*/
public function testCannotCreateOnUnassignedDomain(): void {
// Load created domains in deterministic order.
$domains = \Drupal::entityTypeManager()->getStorage('domain')->loadMultiple();
$domains = array_values($domains);
$this->assertTrue(count($domains) >= 2, 'Two domains are available.');
$domainA = $domains[0];
$domainB = $domains[1];
// Create a user who can create page content only on assigned domains.
$account = $this->drupalCreateUser([
'access content',
'create page content on assigned domains',
]);
// Assign the user only to domain B.
$this->addDomainsToEntity('user', $account->id(), $domainB->id(), DomainAccessManagerInterface::DOMAIN_ACCESS_FIELD);
// Set the active domain to A and log in as the limited user.
/** @var \Drupal\domain\DomainNegotiatorInterface $negotiator */
$negotiator = \Drupal::service('domain.negotiator');
$negotiator->setActiveDomain($domainA);
$this->domainLogin($domainA, $account);
// Try to access node add form on domain A and assert 403.
$this->drupalGet('node/add/page');
$this->assertSession()->statusCodeEquals(403);
}
}
thus un assigning this issue from myself.
Working on the tests and MR.
I tested this and confirmed that the Automator Text Suggestion plugin does not appear for string_long / string_textarea fields until theyâre added to the plugin annotation.
After adding them, the automator works as expected.
Before preparing an MR: should we extend support to all text-related field + widget types (e.g. text_long, text_with_summary, text_textarea_with_summary) at the same time, so coverage is complete? Or keep the MR focused only on string_long / string_textarea?
Made my changes and now the pipelines all green.
Please review!!
The MR had some merge conflicts, so resolved them and also resolved the phpcs errors.
Please review
Altered the test such that the changes related to this issue are not causing any error on the pipeline,
Now the test is only showing errors which already exists on 6.3.x, unrelated to this issue.
So please review.
Now all tests are running green,
Please review!!
Please review!!
Working on it!!
Working on it!!
Converted patch into MR, also added test to verify this,
Please review!!
divyansh.gupta â changed the visibility of the branch 2.x to hidden.
divyansh.gupta â changed the visibility of the branch 3269933-inconsistent-moderation-state to hidden.
divyansh.gupta â changed the visibility of the branch 3269933-8.1.x-inconsistent-moderation-state to hidden.
Working on tests.
Please review!!
Resolved the merge conflicts.
Please review
divyansh.gupta â changed the visibility of the branch 2938731-order-total-range to hidden.
Working on it!!
Re-rolled it @heddn
divyansh.gupta â made their first commit to this issueâs fork.
Ok @smustgrave,
Seen many people do such issue so thought anyone can do these, will keep that in mind from next time!!
Applied the changes and the issue is fixed for me,
The changes also looks good to me,
Thus moving this to RTBC
Please resolve merge conficts.
I have reviewed the issue and now the flagged prompts are also coming in logs.
The changes looks good to me, also the pipeline is green with no merge conflicts.
Thus moving this to RTBC!
Removed yoroy from maintainers.txt,
Please review!
Was able to reproduce the error, also resovled it.
Before:
After:
Please review!!
Working on it!!
Working on it!!
Rebased it @quietone
divyansh.gupta â made their first commit to this issueâs fork.
Rerolled the MR,
Please review!!
Working on it!!
This was happening because $classResolver wasnât declared as a property before being assigned in the constructor. Iâve added the missing property declaration to fix it properly.
Please review!!
Working on it
Rerolled successfully,
Please review!!
Rebased, and also resolved the pipeline errors,
Please review!!
Working on the rebase.
Working on the rebase.
yes i used better exposed filter option only but was unable to reproduce the issue, the RESET button was working absolutely fine for me.
@ahmad khader,
I tried reproducing the error as per the steps you mentioned but the reset button is working absolutely fine for me, but leaving this to active if anyone else is able to reproduce this.
Working on it!!
The data-action-id attribute is correctly rendered on each bulk action button, allowing frontend code to identify which action was triggered. Matches the intent of the issue. Moving to RTBC.
Before:
After:
Reviewing this
Sorry @bohart, for not testing and directly pushing the patch as MR, now i have worked and made all tests pass, also here is an screenshot proving that i have successfully made the key selection using key module.
Please review!!
@bohart,
I have resolved the pipelines errors and now it's all green,
Also i did not test the changes because it thought these were already tested and only the patch was to be converted to MR, do you want me to test the changes?
Thank you!!
Converted patch into MR, also done some changes manually as patch was not applying directly.
Please review!!
Provided the patch as MR,
Please review!!