Chengannur
Account created on 29 November 2019, almost 5 years ago
  • Drupal Back End developer at QED42 
#

Merge Requests

More

Recent comments

🇮🇳India Akhil Babu Chengannur

I have updated the front page template by following the same component structure in 📌 Create MR with SDDS Sections so XB demo can use MR patch Needs review .
This is how the front page looks right now

🇮🇳India Akhil Babu Chengannur

Thanks for the guidance @wim leers. I have updated the validator code. Drupal\Tests\experience_builder\Kernel\ComponentTest::testObsoleteStatusHandling still fails. I am not sure if this test should be removed or updated. Moving to needs review for feedback.

🇮🇳India Akhil Babu Chengannur

I ran across this scenario while exploring the component listing in admin/structure/component

  • By default, the experimental component (components/sdc_statuses/experimental) is listed in the component listing and is active.
  • Change the status of the component from experimental to obsolete in experimental.component.yml
  • The component gets disabled in the component listing (Expected)
  • The component gets added in the 'Disabled components' section (admin/structure/component/status) with reason "Component has "obsolete" status" (Expected)
  • Changed the status back to experimental in experimental.component.yml
  • The component will still be disabled in the component listing
  • The component will still be present in the disabled components section with the same reason "Component has "obsolete" status" . Since the component is disabled, it should be listed under the 'Disabled components' section. But the reason is incorrect. I am not sure what reason to use in this case though. Already, there is a 'Manually disabled' reason. But that's also not quiet right for this scenario.
🇮🇳India Akhil Babu Chengannur

Added a validation constarint to check if 'status' is TRUE.
Now updating the tests

🇮🇳India Akhil Babu Chengannur

Ive added a test for validating the previous revisions. The test creates 3 revisions for the node. The link prop is used in the second revision. So, the exception message should contain '2' as the revision id.
The test works fine in local. But for some reason, revision id is coming as '1' in the exception message in gitlab ci and the test is failing...

🇮🇳India Akhil Babu Chengannur

I tested MR 277 and the media library widget works fine with JS aggregation enabled

🇮🇳India Akhil Babu Chengannur

Just noting down the findings

  • Heading

Should be aligned with the icon cards section

  • Case Study (starshot-case-study2)

Image overlapped with the text as logo, paragraph, and data components are not added to their slots. The components also needs some top margin.

  • CTA (starshot-cta2)

The HTML tags are rendered in front end ascontent_right and content_right_bottom props do not use the |raw filter in template.

  • Stats + Four column in slot + Stats card in each slot

Style is broken when this combination is used

🇮🇳India Akhil Babu Chengannur

I have created the front page with the given components.

Hero (starshot-hero2), Four column with icon cards, and Testimonial components render perfectly while the others have few styling issues

🇮🇳India Akhil Babu Chengannur

Couldn't figure out why the widget is not working with js aggregation enabled :( Maybe something to do with the order in which the js is executed?

🇮🇳India Akhil Babu Chengannur

Thanks @wim leers, I've used the same logic in \Drupal\experience_builder\EventSubscriber\ApiMessageValidator::isProd() to check if assertions are enabled.

  • Should we change this to a common function? (ApiMessageValidator::isProd()) is a private method
  • Should the priority be changed to REQUIREMENT_ERROR if assertions are enabed?
🇮🇳India Akhil Babu Chengannur

Image effects have only uuids. So to remove an effect, we should pass it's uuid to the config action.
I created a config action 'deleteImageEffect' which accepts the uuid of the effect to be removed, as a starting point. Moving this to 'needs review' for feedback. Haven't added any tests for now.

config:
  actions:
    image.style.large:
      deleteImageEffect: 62f7b707-a0cc-459b-a8ee-1dd3eeb9b445
🇮🇳India Akhil Babu Chengannur

akhil babu made their first commit to this issue’s fork.

🇮🇳India Akhil Babu Chengannur

Thanks @smustgrave. Alexpott added a comment in the MR to add a test to verify the valid config actions. Drupal\Core\Config\Action\ConfigActionManager::getShorthandActionIdsForEntityType gives the list of shorthand actions per entity type, but this method is protected. So @alexpot suggested to hardcode the exception message.
https://drupal.slack.com/archives/C1BMUQ9U6/p1724325139885199

🇮🇳India Akhil Babu Chengannur

Updated the test to validate the config action names.

🇮🇳India Akhil Babu Chengannur

The code in XB enforces the 'attribute' prop to be of type Drupal\Core\Template\Attribute
web/modules/contrib/experience_builder/src/PropShape.php

      // TRICKY: `attributes` is a special case — it is kind of a reserved
      // prop.
      // @see \Drupal\sdc\Twig\TwigExtension::mergeAdditionalRenderContext()
      // @see https://www.drupal.org/project/drupal/issues/3352063#comment-15277820
      if ($prop_name === 'attributes') {
        assert($prop_schema['type'][0] === Attribute::class);
        continue;
      }

So, If 'attribute' prop is added with any other type (like type: string for most of the SDDS components), XB UI will break.

🇮🇳India Akhil Babu Chengannur

Thanks @sea2709. The 'notify_new_comments' recipe was there in web/recipes. It's also getting recreated when I run composer install after deleting it.

🇮🇳India Akhil Babu Chengannur

I got these warnings during ddev start step

No composer.lock file present. Updating dependencies to latest instead of installing from lock file

Authentication required (gitlab.lakedrops.com). I just hit enter for username and password and got "TypeError]
rawurlencode(): Argument #1 ($string) must be of type string, null given" error

🇮🇳India Akhil Babu Chengannur

Thanks @thejimbirch. I have opened a new MR against core

🇮🇳India Akhil Babu Chengannur

akhil babu changed the visibility of the branch 3302833-PluginNotFound to hidden.

🇮🇳India Akhil Babu Chengannur

akhil babu changed the visibility of the branch 3365453-cores-minimal-install-profile-as-recipes to hidden.

🇮🇳India Akhil Babu Chengannur

Added the tests.

Also I think this issue can be made against drupal/core and not the recipes initiative as the code is in core.

Not sure how to do it. Create a new issue against Drupal core and close this one?

🇮🇳India Akhil Babu Chengannur

Also, fields like selected, options.options.is_selected, options.options.is_disabled are documented as string fields in the twig file. But shouldn't these be boolean fields? (The logic in the twig confirms the same)

{% if sub_option.is_selected %}selected="selected"{% endif %} 
{% if sub_option.is_disabled %}disabled{% endif %}
🇮🇳India Akhil Babu Chengannur
 *   - options: [array] Array of options (applies to optgroup type):
 *     - label: [string] Option label.
 *     - value: [string] Option value.
 *     - is_selected: [string] Flag whether option is selected.
 *     - is_disabled: [string] Flag whether option is disabled.

The script didnot convert the properties inside the nested options field (Options -> Options). Current output is

options:
  type: array
  title: Options
  description: 'Array of options (applies to optgroup type):'

I think it should have been like

options:
  type: array
  title: Options
  description: 'Array of options (applies to optgroup type):'
  items:
    label:
      type: string
      title: Label
      description: Option label.

    value:
      type: string
      title: Value
      description: Option value.

    is_selected:
      type: string
      title: Selected
      description: Flag whether option is selected.

    is_disabled:
      type: string
      title: Selected
      description: Flag whether option is disabled.
🇮🇳India Akhil Babu Chengannur

Here is the output after running the script

name: Select

status: experimental

group: Atoms

props:

  type: object
  required:
    - options

  properties:
    theme:
      type: string
      title: Theme
      description: 'Theme: light, dark.'
      default: light
      enum:
        - light
        - dark

    is_multiple:
      type: boolean
      title: Is multiple.

    options:
      type: array
      title: Options
      description: 'Options:'
      items:
        type: object

        properties:
          type:
            type: string
            title: Type
            description: 'Option type: option group (optgroup) or option (option).'

          label:
            type: string
            title: Label
            description: Option label.

          value:
            type: string
            title: Value
            description: Option value.

          selected:
            type: string
            title: Selected
            description: Flag whether option is selected.

          options:
            type: array
            title: Options
            description: 'Array of options (applies to optgroup type):'

    is_invalid:
      type: boolean
      title: Denote if the control is invalid.

    is_disabled:
      type: boolean
      title: Denote if the control is disabled.

    is_required:
      type: boolean
      title: Denote if the control is required.

    attributes:
      type: string
      title: Attributes
      description: Additional attributes.

    modifier_class:
      type: string
      title: Modifier class
      description: Additional classes.

🇮🇳India Akhil Babu Chengannur

akhil babu changed the visibility of the branch 3302833-imporve-exception-2 to hidden.

🇮🇳India Akhil Babu Chengannur

akhil babu changed the visibility of the branch 3302833-imporve-exception to hidden.

🇮🇳India Akhil Babu Chengannur

Here are the steps

Prerequisites

  • Make sure you have Python installed on your system
  • Check that the required libraries (pyyaml and re) are available in your Python environment.

Downloading the script

  • Follow the SDDS development guide to install Drupal Starshot with the Starshot Demo Design System theme
  • Go to the project folder. If you have followed the steps in the SDDS development guide, this will be the 'starshot' folder
  • To download the script, run git clone https://github.com/qed42/twig-sdc-yaml-generator.git . This will download the folder 'twig-sdc-yaml-generator' containing the script.

Executing the script

  • The script uses the 'comments' in the component twig files to generate the props and slots. Few of the SDDS components have incorrect comments in their twig files. Those should be adjusted before running the script'. For that
  • Structure of the command: python3 
  • Move to the project folder: cd starshot
  • Converting a single component
    • python3 twig-sdc-yaml-generator/twig_sdc_yaml_generator.py web/themes/contrib/demo_design_system/components/01-atoms/button/
    • The above command will generate button.component.yml and README.md files in web/themes/contrib/demo_design_system/components/01-atoms/button/ folder.
  • Converting a group of components
    • python3 twig-sdc-yaml-generator/twig_sdc_yaml_generator.py web/themes/contrib/demo_design_system/components/01-atoms/
  • Converting all components at once
    • python3 twig-sdc-yaml-generator/twig_sdc_yaml_generator.py web/themes/contrib/demo_design_system/components
🇮🇳India Akhil Babu Chengannur

Noticed a tiny issue. 's' is not hyperlinked in 'settings'.

🇮🇳India Akhil Babu Chengannur

I also tried to render 2 of the existing components and these are the results.

Snippet

  • Updated paragraph--civictheme-snippet.html.twig to render the components using SDC. {% embed 'civictheme:snippet' %}{% endembed %}
  • Created a new 'Page' content and added a 'Snippet' paragraph.
  • The component gets rendered correctly
🇮🇳India Akhil Babu Chengannur

The script checks the comment in twig files to determine the props and sots. These changes were required for the script to work.
components/03-organisms/webform/webform.twig

  • The referenced_webform variable randers string value. But, it's type was documented as 'webform'. Changed this to string.

components/03-organisms/navigation/navigation.twig

  • Corrected the spelling of 'boolean'.

components/02-molecules/video-player/video-player.twig

  • Documented the properties of transcript_link

components/02-molecules/logo/logo.twig

  • Adjusted the indentation.
🇮🇳India Akhil Babu Chengannur

Copying my comments from slack

Faced some issues while setting up local site (Drupal 10.3, not the Starshot prototype), following the developer guide. Posting them here along with the workarounds followed
ddev composer require drupal/demo_design_system
Error: Could not find a version of package drupal/demo_design_system matching your minimum-stability (stable)
Workaround: First run ddev composer config minimum-stability dev , then ddev composer require drupal/demo_design_system

drush ev "require_once dirname(\Drupal::getContainer()->get('theme_handler')->rebuildThemeData()['civictheme']->getPathname()) . '/theme-settings.provision.inc'; civictheme_enable_modules();"
Error: Unable to install modules due to missing modules search_api, search_api_db
Workaround: Not sure why the search_api module is a dependency. For now I manually downloaded both the modules (search_api and search_api_db)

ddev drush then demo_design_system
Error: Unknown themes: demo_design_system
Workaround: The theme files are still in the format civictheme.*.yml . So used ddev drush then civictheme to install the theme.

But, Overall the SDDS documentation is really great and provides a good understanding of the initiative, especially the contribution and SDC conversion guides.

🇮🇳India Akhil Babu Chengannur

This worked for me

$view->initPager();
$view->pager->setItemsPerPage(10);
🇮🇳India Akhil Babu Chengannur

I tested this in my local and it works perfectly. However no errors or warnigs were shown for invalid configurations like

input:
  recipient:
    source: config
    config: ['system.invalid_config', 'mail']

Should we add a check to verify if the config exists or not?

🇮🇳India Akhil Babu Chengannur

Added a Kernel test for the example recipe as suggested in #24. Also updated simple_config_update to simpleConfigUpdate in the example recipe.

🇮🇳India Akhil Babu Chengannur

Akhil Babu made their first commit to this issue’s fork.

🇮🇳India Akhil Babu Chengannur

This would be very helpfull. Right now, Drupal just shows a 'page not found' message when the route is invalid.

🇮🇳India Akhil Babu Chengannur

This issue is already being discussed in 🐛 Project detail page is throwing an exception Needs review and there is an MR with the same fix.

🇮🇳India Akhil Babu Chengannur

Changing the Priority to minor as there is no significant impact on the functionality. I have opened a merge request. Please review

🇮🇳India Akhil Babu Chengannur

Akhil Babu changed the visibility of the branch 3461340-search-results-number to hidden.

🇮🇳India Akhil Babu Chengannur

Added documentation for the recipe types mentioned in #3, based on the recipes available in core. The 'Responsive image' recipe type preset in the existing documentation has the 'standard_responsive_images' recipe as example . But that recipe has type as 'Media'. Also, there are no recipes with type 'Responsive image'. Should we change the type of 'standard_responsive_images' to 'Responsive image'?

🇮🇳India Akhil Babu Chengannur

Also, adding !\Drupal::isConfigSyncing() condition in shortcut_themes_installed breaks the standard profile recipe test. \Drupal::isConfigSyncing() is true while applying the standard profile recipe. Sothird_party_settings.shortcut.module_link value in claro.settings is never set to true.

🇮🇳India Akhil Babu Chengannur

Tried to reproduce this issue, but couldn't. These are the things that I've tried so far in fresh Drupal 10.3 instance (standard profile)

  • Set third_party_settings.shortcut.module_link value in claro.settings to FALSE
  • Exported the configuration
  • Uninstalled the shortcut module
  • Imported the configuration
  • The value of third_party_settings.shortcut.module_link should have changed to TRUE as per the hook_install code in the module. But the value after config sync was still FALSE
  • Set third_party_settings.shortcut.module_link value in claro.settings to FALSE
  • Exported configuraion
  • Changed the default admin theme to stark
  • Imported the configuration
  • The value of third_party_settings.shortcut.module_link should have changed to TRUE as per the hook_install code in the module. But the value after config sync was still FALSE

So adding 'Needs steps to reproduce' tag.

🇮🇳India Akhil Babu Chengannur

Akhil Babu changed the visibility of the branch 2980951-permission-to-see to hidden.

Production build 0.71.5 2024