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
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.
kristen pol → credited akhil babu → .
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
toobsolete
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.
Added a validation constarint to check if 'status' is TRUE.
Now updating the tests
akhil babu → made their first commit to this issue’s fork.
Unassigning for now
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...
I tested MR 277 and the media library widget works fine with JS aggregation enabled
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
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
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?
Thanks @wim leers. Committed the changed
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?
Added the warning.
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
akhil babu → made their first commit to this issue’s fork.
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
akhil babu → created an issue.
kristen pol → credited akhil babu → .
Updated the test to validate the config action names.
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.
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.
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
Thanks @thejimbirch. I have opened a new MR against core
akhil babu → changed the visibility of the branch 3302833-PluginNotFound to hidden.
akhil babu → changed the visibility of the branch 3365453-cores-minimal-install-profile-as-recipes to hidden.
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?
nod_ → credited akhil babu → .
akhil babu → changed the visibility of the branch 1156338-fixed-maximum-number to active.
akhil babu → changed the visibility of the branch 1156338-fixed-maximum-number to hidden.
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 %}
* - 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.
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.
These are the changes
akhil babu → changed the visibility of the branch 3302833-imporve-exception-2 to hidden.
akhil babu → changed the visibility of the branch 11.x-1 to hidden.
akhil babu → changed the visibility of the branch 11.x to hidden.
I tried to create an MR for this, but I am not able to rebase it properly
These are the changes: https://git.drupalcode.org/project/distributions_recipes/-/merge_request...
akhil babu → changed the visibility of the branch 3302833-imporve-exception to hidden.
akhil babu → made their first commit to this issue’s fork.
Updated the IS, Thanks
I have added the tests. Ready for review
akhil babu → made their first commit to this issue’s fork.
Kristen Pol → credited Akhil Babu → .
Kristen Pol → credited Akhil Babu → .
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
-
Go to the SDDS theme folder
cd web/themes/contrib/demo_design_system
-
Run thw command
curl -sL https://git.drupalcode.org/project/demo_design_system/-/merge_requests/3/diffs.patch | patch -p1
- To manually make these adjustments without using a patch, check the merge request https://git.drupalcode.org/project/demo_design_system/-/merge_requests/3/diffs
-
Go to the SDDS theme folder
-
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
-
Kristen Pol → credited Akhil Babu → .
Noticed a tiny issue. 's' is not hyperlinked in 'settings'.
Akhil Babu → created an issue.
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
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.
Akhil Babu → made their first commit to this issue’s fork.
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.
This worked for me
$view->initPager();
$view->pager->setItemsPerPage(10);
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?
Added a Kernel test for the example recipe as suggested in #24. Also updated simple_config_update to simpleConfigUpdate in the example recipe.
Akhil Babu → made their first commit to this issue’s fork.
This would be very helpfull. Right now, Drupal just shows a 'page not found' message when the route is invalid.
Akhil Babu → made their first commit to this issue’s fork.
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.
Changing the Priority to minor as there is no significant impact on the functionality. I have opened a merge request. Please review
Akhil Babu → changed the visibility of the branch 3461340-search-results-number to hidden.
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'?
Akhil Babu → made their first commit to this issue’s fork.
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.
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 inclaro.settings
toFALSE
- Exported the configuration
- Uninstalled the shortcut module
- Imported the configuration
- The value of
third_party_settings.shortcut.module_link
should have changed toTRUE
as per the hook_install code in the module. But the value after config sync was stillFALSE
- Set
third_party_settings.shortcut.module_link
value inclaro.settings
toFALSE
- Exported configuraion
- Changed the default admin theme to stark
- Imported the configuration
- The value of
third_party_settings.shortcut.module_link
should have changed toTRUE
as per the hook_install code in the module. But the value after config sync was stillFALSE
So adding 'Needs steps to reproduce' tag.
Added tests.
Akhil Babu → made their first commit to this issue’s fork.
Akhil Babu → changed the visibility of the branch 2980951-permission-to-see to hidden.