🇮🇳India @adwivedi008

Test the MR-8 changes with CKE-5 and Drupal-10 and its working fine as the feature is expected

Moving the issue to RTBC

Merged D-11 related changes

Hi everyone, Thanks for working on the issue
Can someone create a merge request with the latest changes

I am facing the same issue and patch #50 doesn't seem to resolve the issue for D-10.2.6
any suggestions

@rveillard Do you find any solution for this
I am also getting the same issue for video component

@jochim updated the MR as per your suggestions
Apology for the bulk changes as I get confused about bringing the whole doc changes vs only document part changes

Moving the issue to needs review.

@joachim
I have updated the documentation using https://www.drupal.org/files/issues/2020-05-29/3040427-37-migrate_plus_m... →
and https://www.drupal.org/files/issues/2020-05-17/3040427-32-migrate_plus_m... → as reference as you have mentioned in #5

Closed MR of #7 as that was not correct and was opened by mistake apology for that

Please review the changes and inform if any other changes are required

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

Hi everyone,

I want to work on this issue can someone guide me to which section can we add the module and remove from core

Created MR for all the issues raised over coding standard command
Checked using the following command.
vendor/bin/phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,info,txt,md,yml ./

Moving the issue to Needs Review

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

Verified the changes mentioned on MR-7857
The padding is 12px 16px over the menu items child

Attaching a screenshot as a reference

Moving the issue to RTBC

RTBC++

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

@finnsky Can confirm the issue only happens when the inspect element is open and the toolbar gets a scrollbar on it

I am attaching a screen record as a reference and looking into this issue.

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

@ckrine @m4olivei

Implemented the suggested changes but got Merge error
Can we resolve this by rebasing with 11.x

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

Rebased '3443567-update-ckeditor-5-11.x' branch with 11.x

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

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

Reverted the remaining issue.

Setting the issue to Needs review

Reverted out-of-scope changes suggested by @smustgrave on #14

Setting the issue to Needs review

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

Rebased the branch with 11.x
Please review if any other changes are required.

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

Patch at #8 works great for 9.5.x and 10.1.x but it failing for D-10.2.x

@smustgrave I was not sure about the permissions that's why I haven't worked on it.

Thanks @spuky

Working fine on D-9.5

Any update on this for D-9.5 or D10

Worked on Pt. 1 mentioned in the proposed solution.

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

Moving the issue back to Need Review as it was already in Needs review state

Updated the MR-5052
Rebased to 11.x

Please review if any other changes are required.

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

I have added a new configuration key to the config form of the block to show and hide the count of searches
Attaching a screenshot for reference

Moving the issue to Needs Review.

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

@adarshv Tested the MR-3 against D-10 but it still needs work as the header is still broken comparatively
So moving the issue to needs work
Attaching a screenshot for reference.

Tested the MR-12 and it fixes the issue
Attaching the screenshot as a reference.

Moving the issue to RTBC

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

@wim fixed the phpcs issue and updated the MR PLease review

Moving the issue to needs review

adwivedi008 → changed the visibility of the branch 3440878-fix-return-typehinting to active.

adwivedi008 → changed the visibility of the branch 3440878-fix-return-typehinting to hidden.

Created MR with the proposed solution over LangcodeRequiredIfTranslatableValuesConstraintValidator::validate()
Please review
Moving the issue to Needs review.

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

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

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

Hi @alex.skrypnyk
Thanks for raising the issue and proposing solution

Implemented the proposed solution and created MR
It needs review so moving the issue to needs review

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

MR #28 seems fine to me and it applies cleanly
also changing the icon so moving the issue to RTBC

I tried to reproduce the issue and it seems to be fixed on the latest release of the module
So RTBC++ from my side.

#MR! 4 seems fine to me

so RTBC+1 from my side.

Checked the MR mentioned at #3 and the changes look fine to me

Moving the issue to RTBC

Able to reproduce the issue reported by @Charles Belov
Fixed the issue and also added the screenshot of the before and after issue

Moving the issue to needs review.

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

Revised #319 as it cleanly applies for D-10.2.x but was facing an issue with the pencil icon on the layout translation page.

Fixed the issue.

Please review and suggest if any other change is required

Resolved the coding standard issue at last Pr due to which the pipeline was failing and setting back the status to RTBC from Needs works

Please review and suggest if any other changes are required.

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

Manually Tested the MR and confirm that it resolved the issues regarding the Cancel and Confirm button functionality so RTBC+ From my side.

Changing the issue's status to RTBC, so that other people can also review and provide their suggestions.
As mentioned by @rkoller not very sure about the test coverage.

Updated the description of $element['uri']['#description'] as well. The description was also using http://example.com, converted it to HTTPS,

Query: Found the same issue in code comments should we create another issue for them or resolve it under the same issue?

Moving the issue to needs review, Please review and inform if any other changes are required.

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

MR at #123 seems fine but as on comparison we can see a lot of changes at the MR
Moving the issue to needs review as its previous status was RTBC over #120

Let's wait for some more reviews and then move this to RTBC

RTBC+ From my side

MR at #09 seems good to me, it also passes test cases over the pipeline.
Moving the issue to RTBC.

@smustgrave Please review and share if any other change is required.

Tested the coding standard issue using the following command

vendor/bin/phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig  ./

Seems MR #12 resolved the coding standard issue

So moving the issue to RTBC.

@aaron.ferris I think hiding the reset button won't work.

In certain cases, we can hide it but from the user's perspective it would be not very clear if the button is visible at certain places and hidden from certain places

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

Replicated the issue at D-10 Seems MR at #2 Resolves the issue mentioned

Adding screenshot before and after the MR

Moving the issue to RTBC.

Tested the issue mentioned and MR at #5 fixes the issue
Moving the issue to RTBC and attaching screenshot.

Added readme.md file as following the https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or... →

Please review and suggest if any other changes are required.

Moving the issue to needs review.

Thank you @quietone for addressing the issue
The MR #2 looks good to me So RTBC +1 from my side

let's wait for some more reviews

Simply apply the changes suggested by @seanB over #13

Used $is_owner = $account->id() && (int) $account->id() === (int) $entity->getOwnerId();
to check the access.
Please review and suggest if any other changes required

Moving the issue to needs review

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

I am working on readme file and will update this

I tested the MR mentioned in #4, and it fixes the issue
So moving the issue to RTBC

@Hamid.ali By mistake i posted the wrong patch

Thank You for testing,
Here is the correct patch Please test

Moving the issue to needs review

Resolved the issue attaching the screenshot
Please verify
Moving the issue to needs review

adwivedi008 → changed the visibility of the branch 2.0.x to active.

adwivedi008 → changed the visibility of the branch 2.0.x to hidden.

Resolved the following coding standards

Moving the issue to needs review.

adwivedi008 → created an issue.

@nathanhealea resolved the issue using the theme hook pre-processor-page()
Please review
https://git.drupalcode.org/project/insert_view_adv/-/merge_requests/3
Moving the issue to Needs-review

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

this has been tested

I applied the patch mentioned in #6 and it is working as expected
Moving the issue to RTBC

@joachim fixed the Wrapping issue and resolved other issues reported over PHP-CS/CBF
Please review

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

Fixed a few remaining issues but some issues remain
The remaining issues are:

FILE: /eck.module
-------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
-------------------------------------------------------------------------------------
 27 | WARNING | Global constants should not be used, move it to a class or interface
 32 | WARNING | Global constants should not be used, move it to a class or interface
-------------------------------------------------------------------------------------

FILE: src/Form/EntityType/EckEntityTypeFormBase.php
----------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
----------------------------------------------------------------------------------------------
  93 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 157 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
----------------------------------------------------------------------------------------------

FILE: /src/Form/EntityBundle/EckEntityBundleForm.php
----------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 4 WARNINGS AFFECTING 4 LINES
----------------------------------------------------------------------------------------------
 110 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 215 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 218 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 281 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
----------------------------------------------------------------------------------------------

FILE: /README.md
----------------------------------------------------------------------
FOUND 0 ERRORS AND 4 WARNINGS AFFECTING 4 LINES
----------------------------------------------------------------------
 3 | WARNING | Line exceeds 80 characters; contains 84 characters
 4 | WARNING | Line exceeds 80 characters; contains 85 characters
 7 | WARNING | Line exceeds 80 characters; contains 85 characters
 8 | WARNING | Line exceeds 80 characters; contains 85 characters
----------------------------------------------------------------------

FILE: /tests/src/Kernel/EckEntityTest.php
-------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------------------
 42 | ERROR | The array declaration extends to column 82 (the limit is 80). The array content should be split up over multiple lines
-------------------------------------------------------------------------------------------------------------------------------------

FILE: /tests/src/Unit/UnitTestBase.php
----------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
----------------------------------------------------------------------------
 51 | WARNING | Unused variable $methods.
 81 | WARNING | Unused private method getEntityManagerMock()
----------------------------------------------------------------------------

I have used the following command for checking the Drupal-PHP-CBF/CS issues

vendor/binphpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,info,txt,md,yml ./

Please review

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

Worked on #6 as suggested by @karanpagare
Moving the issue to needs review

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

hi @jaypan can you please give some example for that 

I am using a custom field for comment on custom entity 

Patch #65 works for me i am using it on D-9.5

Rerolled patch#237 as it was failing for D-10.1.7

Replaced the enable to install

core/modules/language/language.module:

$additional_overview = t("If the Interface Translation module is enabled, this page will provide an overview of how much of the site's interface has been translated for each configured language.");
 $additional_continue = t('Depending on your site features, additional modules that you might want to enable are:') . '<ul>';

while the second one is already resolved.
core/modules/system/system.install:

'description' => t('The current database driver is provided by the module: %module. The module is currently not enabled. You should immediately <a href=":enable">enable</a> the module.', ['%module' => $provider, ':enable' => Url::fromRoute('system.modules_list')->toString()]),

Please review if any other changes needed, moving the ticket to needs review.

Revised #239 for

PHP Fatal error: Class Drupal\layout_builder\Section contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (JsonSerializable::jsonSerialize) in /web/core/modules/layout_builder/src/Section.php on line 21

After comparing with #251