- Issue created by @tomsaw
- Status changed to Needs review
over 1 year ago 8:50am 4 June 2023 - last update
over 1 year ago Patch Failed to Apply - last update
over 1 year ago 2 pass - π¦πΊAustralia larowlan π¦πΊπ.au GMT+10
How about this simpler approach instead?
- π©πͺGermany tomsaw Essen
1 lvl less nesting! I think that's better.
We may take the chance and fix empty settings behavior for "Pattern settings" and "Context variables" as well?
These are not critical (do not throw exceptions) but you get empty, confusing 'details' containers in the settings form when nothing is provided. - last update
over 1 year ago 2 pass - Status changed to Needs work
over 1 year ago 8:48pm 12 June 2023 - π¦πΊAustralia larowlan π¦πΊπ.au GMT+10
Thanks for working on this
-
+++ b/src/Plugin/Block/ComponentBlock.php @@ -277,56 +277,58 @@ class ComponentBlock extends BlockBase implements ContainerFactoryPluginInterfac + if(isset($plugin['fields']) && !empty($plugin['fields'])) {
!empty($plugin['fields']) should be enough here (don't need both empty and isset)
-
+++ b/src/Plugin/Block/ComponentBlock.php @@ -337,12 +339,14 @@ class ComponentBlock extends BlockBase implements ContainerFactoryPluginInterfac - $form['settings'] = array_merge($form['settings'] ?? [], [ - '#type' => 'details', - '#title' => $this->t('Pattern settings'), - '#open' => TRUE, - '#tree' => TRUE, - ]); + if(isset($form['settings'])) { + $form['settings'] = array_merge($form['settings'], [ + '#type' => 'details', + '#title' => $this->t('Pattern settings'), + '#open' => TRUE, + '#tree' => TRUE, + ]); + }
I don't think we need this, we already had null coalesce here
-
- last update
over 1 year ago 4 fail - @tomsaw opened merge request.
- last update
over 1 year ago 4 fail - last update
over 1 year ago PHPLint Failed - π©πͺGermany tomsaw Essen
Thanks for mentoring!
Never got familiar with patch-workflow. There's more to come in my contribution career, thus made myself familiar with Drupal PR workflow on drupalcode.org Lets continue there!
- last update
over 1 year ago 4 fail - @tomsaw opened merge request.
- π©πͺGermany tomsaw Essen
Oops.. i've ended with 2 issue branches and 2 merge request. Sorry for the noise. Just drop the wrong instance. Next time I'll do better.
- last update
about 1 year ago 4 fail - π©πͺGermany tomsaw Essen
@larowlan please have a look again. This MR should do it now!
- Status changed to Needs review
about 1 year ago 3:26pm 2 November 2023 - last update
about 1 year ago 2 pass - Status changed to Needs work
about 1 year ago 12:51am 9 November 2023 - π¦πΊAustralia larowlan π¦πΊπ.au GMT+10
Looks like tests are failing?
- π©πͺGermany tomsaw Essen
obviously I do not have enough comprehension how drupal and gitlab works.
Back to school for me then... I'll come back with a nice green MR.