Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DS-1656] Add a new field 'Alert Style' for Alert nodes #146

Merged
merged 4 commits into from
Nov 22, 2024

Conversation

aleevas
Copy link
Contributor

@aleevas aleevas commented Nov 11, 2024

The "How to check" instructions are included in the Jira issue DS-1656 .

@aleevas aleevas force-pushed the DS-1656 branch 5 times, most recently from 48ad0fb to c8775b4 Compare November 19, 2024 09:41
@aleevas aleevas changed the title [DS-1656] WIP [DS-1656] Add a new field 'Alert Style' for Alert nodes Nov 20, 2024
/**
* Implements hook_form_FORM_ID_alter().
*/
function openy_node_alert_form_node_alert_edit_form_alter(array &$form, FormStateInterface $form_state, $form_id) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's apply the DRY principle here and avoid code duplication.
Something like:

  $fields_to_toggle = [
    'field_alert_color',
    'field_alert_text_color',
    'field_alert_icon_color',
  ];

  foreach ($fields_to_toggle as $field_name) {
      $form[$field_name]['widget']['#states'] = [
        'visible' => [
          ':input[name="field_alert_style"]' => ['value' => 'classic'],
        ],
      ];
  }

Comment on lines 420 to 437
$configs = [
'field.field.node.alert.field_alert_color' => [
'required',
],
'field.field.node.alert.field_alert_icon_color' => [
'required',
],
'field.field.node.alert.field_alert_text_color' => [
'required',
]
];
$config_updater = \Drupal::service('openy_upgrade_tool.param_updater');
foreach ($configs as $config_name => $params) {
$config = $config_dir . $config_name . '.yml';
foreach ($params as $param) {
$config_updater->update($config, $config_name, $param);
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OPTIONAL:
Can also be more compact:

  $configs = [
    'field.field.node.alert.field_alert_color',
    'field.field.node.alert.field_alert_icon_color',
    'field.field.node.alert.field_alert_text_color',
  ];

  foreach ($configs as $config_name) {
    $config_file = $config_dir . $config_name . '.yml';
    $config_updater->update($config_file, $config_name, 'required');
  }

@rollki
Copy link

rollki commented Nov 20, 2024

Thanks @aleevas
Good job, just a small comment from me.

Copy link

@rollki rollki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@aleevas aleevas marked this pull request as ready for review November 20, 2024 13:35
Copy link
Contributor

@podarok podarok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small fixes needed

@podarok podarok merged commit 2815605 into open-y-subprojects:main Nov 22, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants