[SD-22]Updated hero image theme according header style

[yeniatencio]

Jira

https://digital-vic.atlassian.net/browse/SD-22
BE testing link: https://nginx-php.pr-1711.content-vic.sdp4.sdp.vic.gov.au/

Problem/Motivation

We want to have these scenarios for customised header:

1. Corner graphics - Light + Dark (Both options available and by default Light should be selected),
2. Full background image - Dark only (Only one option should be set)
3. CTA - Light only (Only one option should be set).
4. Default appearance - Light only (Only one option should be set)

Fix

1. Set field field_landing_page_hero_theme to mandatory.
2. Remove a patch for drupal/jsonapi_extras as new released was done and patch is not needed anymore.
3. Fix typo in label : Here image theme.
4. Updated behat test.
5. Updated tide publication module to make it work in publication and publication page content type.

Related PRs

Screenshots

TODO

[MdNadimHossain]

@yeniatencio use states instead of custom JS. See this example - https://github.com/dpc-sdp/tide_core/blob/develop/modules/tide_landing_page/tide_landing_page.module#L218

[MdNadimHossain]

We do not want use js if it is can be done without it. Read the SD, I have mentioned about conditional logic. You will find other examples in landing page where we have used conditional logic states. Take example from there and try to work it out using states instead of adding this JS please.

[yeniatencio]

@MdNadimHossain agreed. I've already using conditional logic for enable/disable the field_landing_page_hero_theme as per the requirements. However, one of the requirements is to set the default value of one field based on the value of another field and the only Drupal States that can be applied to a form field element are:

• enabled
• disabled
• required
• optional
• visible
• invisible
• checked
• unchecked
• expanded
• collapsed

As you can notice the default value can't be changed using states. Also, all the examples in landing page module is for changing one of the states in the list above but a default value.
Therefore, that's why I ended up writing JS code for it. Hope it makes sense now.

[MdNadimHossain]

@yeniatencio I have suggested some suggestions. Please have a look into the revised version and it should work in the same way -

(function (Drupal) {
  "use strict";

  Drupal.behaviors.heroImageTheme = {
    attach: function (context, settings) {
      const imageTheme = context.querySelector(
        'select[data-drupal-selector^="edit-field-landing-page-hero-theme"]'
      );

      if (imageTheme !== null) {
        setImageTheme();

        const headerStyles = context.querySelectorAll(
          'input[data-drupal-selector^="edit-header-style-options"]'
        );

        if (headerStyles.length > 0) {
          headerStyles.forEach((style) => {
            style.addEventListener("change", setImageTheme);
          });
        }
      }

      function setImageTheme() {
        const selectedHeaderStyle = document.querySelector(
          'input[data-drupal-selector^="edit-header-style-options"]:checked'
        );

        if (!selectedHeaderStyle) {
          return; // Exit if no style option is selected.
        }

        let defaultHeaderStyle = selectedHeaderStyle.value;
        imageTheme.value = defaultHeaderStyle === "fullwidth" ? "dark" : "light";
      }
    },
  };
})(Drupal);

[MdNadimHossain]

Use context to ensure that the behavior only attaches to elements in the current page context.

[MdNadimHossain]

The check if (headerStyles !== null) should instead check for headerStyles.length > 0. It’s better to check if any elements are found before adding listeners.

[MdNadimHossain]

Add a guard clause to prevent this error in case no radio buttons are checked

[MdNadimHossain]

Instead of repeatedly defining an anonymous function for style.addEventListener("change", ...), you can directly pass the reference to the setImageTheme function.

[MdNadimHossain]

The script accesses document multiple times. It’s a good practice to minimize global object access by caching elements that you reuse.

[MdNadimHossain]

I would suggest moving the setImageTheme() function outside the main attach function for clarity and better separation of logic.

[yeniatencio]

@yeniatencio I have suggested some suggestions. Please have a look into the revised version and it should work in the same way -

(function (Drupal) {
  "use strict";

  Drupal.behaviors.heroImageTheme = {
    attach: function (context, settings) {
      const imageTheme = context.querySelector(
        'select[data-drupal-selector^="edit-field-landing-page-hero-theme"]'
      );

      if (imageTheme !== null) {
        setImageTheme();

        const headerStyles = context.querySelectorAll(
          'input[data-drupal-selector^="edit-header-style-options"]'
        );

        if (headerStyles.length > 0) {
          headerStyles.forEach((style) => {
            style.addEventListener("change", setImageTheme);
          });
        }
      }

      function setImageTheme() {
        const selectedHeaderStyle = document.querySelector(
          'input[data-drupal-selector^="edit-header-style-options"]:checked'
        );

        if (!selectedHeaderStyle) {
          return; // Exit if no style option is selected.
        }

        let defaultHeaderStyle = selectedHeaderStyle.value;
        imageTheme.value = defaultHeaderStyle === "fullwidth" ? "dark" : "light";
      }
    },
  };
})(Drupal);

Hi @MdNadimHossain , Thanks for the suggestions. I have updated my pr as you requested.