🇦🇺Australia @alex.skrypnyk

The Custom date is an override. If it is not selected by the user, a published or updated date should be used instead.

Promo or any other cards/snippets/etc are just the ways to display the referenced content. IMO, the logic should be consistent across all components, but configurable per content type.

So, for "News", for example, a site owner may chose "Updated date" (and Custom date would override it if set by the user)
But, for "Events", for example, a site owner may chose "Created date" (and Custom date would override it if set by the user)

@vladimiraus
I see you added CI that is now reporting issues that have been present in the module for a long time.
Is it possible to do the D11 upgrade as-is and fix those code standards violations in another issue?

richardgaunt → credited alex.skrypnyk → .

Patch attached

This is absolutely hardcoded (pun intended)
https://git.drupalcode.org/project/easy_breadcrumb/-/blob/2.x/src/EasyBr...

   $url_obj = Url::fromUserInput($url, ['absolute' => TRUE]);

Could someone please advise if there is a reason to make that line to be hardcoded and not based on the EB's config setting.

@richardgaunt
The issue is with a `title` attribute being passed into the `button`, but the button does not support that attribute.

The solution is to fix the passing of the attribute:

From

<div class="ct-back-to-top" data-component-name="back-to-top" data-scrollspy data-scrollspy-offset="400">
  {% include '@atoms/button/button.twig' with {
    kind: 'link',
    type: 'primary',
    icon: 'up-arrow',
    url: '#top',
    title: 'Return focus to the top of the page',  <--- BROKEN LINE
    modifier_class: 'ct-back-to-top__button',
  } only %}
</div>

to

<div class="ct-back-to-top" data-component-name="back-to-top" data-scrollspy data-scrollspy-offset="400">
  {% include '@atoms/button/button.twig' with {
    kind: 'link',
    type: 'primary',
    icon: 'up-arrow',
    url: '#top',
    attributes: 'title="Return focus to the top of the page"', <----FIXED HERE
    modifier_class: 'ct-back-to-top__button',
  } only %}
</div>

The button component does not have the `title` prop exposed on purpose: it is not possible to predict what attributes would be needed so exposing only some of them is inconsistent.

Please consider reverting the commit in favour of the proposed fix.

richardgaunt → credited alex.skrypnyk → .

@thejimbirch
could you please provide more information on when and how you got that error.

I just tested on TwigCSFixer 3.0.1, 3.0.3, 3.1.0 on D11.0.4 with and without $config->addTokenParser(new Drupal\Core\Template\TwigTransTokenParser()); on the following code

{% trans %} Submitted by {{ author.name }} on {{ node.date }} {% endtrans %}
{% trans %}Submitted by %author.name% on %node.date%{% endtrans %}

and it passed in all cases without issues.

larowlan → credited alex.skrypnyk → .

I've debugged this and the problem is that the list of links does not receive the 'contextual-links' class. If this class is added - everything works (you can try this by editing a <ul> element with Contextual links in the Web Inspector console and adding the 'contextual-links' class).

It does not receive the class because CivicTheme does in `civictheme_convert_attributes_to_modifier_class()` function:

// Remove class from attributes to avoid duplicated 'class' attribute on
// the element.
unset($variables['attributes']['class']);

As it can be seen in the comments, the intention here was to remove duplicated classes: for modules and themes classes are added via `$variable['classes']`, some through `$variables['attributes']['class']` etc; this function tried to unify this and convert to a UIKit's `modifier_class` property - this property is used in all component to set a CSS class. It was deliberately separated from unexpected use of attributes like `class`, `classes` and `attributes` - try searching the Drupal core and contrib for `{{ class` and you will see that some use `{{ class }}`, some `{{ classes }}` and some `{{ attributes}}`) The problem lies in the Drupal API that treats classes as special attributes, allowing developers to use both `{{ classes }}` and `{{ attributes }}` in their Twig templates.

So, this function cannot predict how core, module and theme developers are going to build their twig templates. This is why CivicTheme uses an opt-in mechanism - to guarantee that visual consistency until a module or theme are explicitly opting in to have their classes correctly specified in the template. The opting-in itself could be in a form of a custom template override which uses `modifier_class` variable directly within `class` attribute on the HTML element OR a preprocess that sets `$variables['modifier_class'] = FALSE`, which disables removal of `$variables['attributes']['class']` value.

I hope this clarifies why CivicTheme strips away classes. Now back to the problem.

The Contextual links are rendered using the `links.twig.html` which are relying on attributes:

<ul{{ attributes }}>

---

Possible solutions
1. `links.twig.html` - override within CivicTheme to use a custom component consisting of the UIKit's menu/navigation, which use `modifier_class`.
OR
2. Add preprocess for the contextual links and set `$variables['modifier_class'] = FALSE`. This will prevent the processing.
OR
3. Remove the `unset($variables['attributes']['class']);` and hope that duplicated classes will not break websites (they will as history has shown).

I am reopening this issue because it differs from Drupal issue #3027640 🐛 Empty strings ("") passed in as contextual filter argument aren't considered missing Needs work , which addresses passing empty values to views arguments in general. This issue specifically concerns the default value not being applied when the value is empty, as described.

The patches provided in the other issue do not resolve this one. I have attached a patch that specifically addresses this issue.

To clarify, if you have a URL like /search?keywords= and a fallback set in the views exposed form, the current getArgument() method in the QueryParameter class does not consider a key/value pair without a value as empty, and therefore does not use the default value.

The RFC 3986 does not specify whether key/value pairs in the URL query without a value should be processed by consumers. However, \Symfony\Component\HttpFoundation\Request does include query parameters without values in its array, which are later used in views to check the presence of each parameter.

In this context, when the exposed filter states "The fallback value to use when the above query parameter is not present," the term "present" could mean either that "the parameter key" is present or that "both the key and value" are present.

I believe the option is intended to set a default value when no value is provided. Therefore, a query string parameter without a value should be considered "not present" in this case.

Implemented and released in 1.8.1

Cards can be found under `Molecules>List`

The `Version` field purpose from https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-... →

Normally, set this to the current development version of the project, rather than the exact release version where you noticed the problem. For Drupal Core, see also the page on backport policy.

This is puzzling me, because this means that all of the issues would need to have their version field changed every time we release. But I will comply.

@sonnykt
thank you for a detailed response. I really appreciate it.

Just a note before the reply: $variables['modifier_class'] = FALSE; is already available to preserve the stripping-out of classes, so this functionality can be disabled by a developer. If this does not work technically - this is a defect.

Conceptually

It's up to the site owner to decide whether to repurpose an existing CivicTheme component, extend a component, develop a new component, or even ignore the UIKit rules and go with a custom styling.

I agree in principle. However, "the site owner" is not always a developer - sometimes it is a script within an automation that just launches the site, sometimes it is a human evaluator without any technical skills. So what we are really talking about here is a opt-in vs opt-out. CivicTheme leans towards having a stable presentation layer out-of-the-box and only supports the components that are available in the UIKit.
The reason why this decision was made is quite simple: more than once contribs has broken the front-end by doing something that should not be doing at all! This has resulted in affecting many-many sites.

So, instead of opt-in to use contribs' classes by default and fix it per-site, we are opting-out by default (to only use CivicTheme's components) and fixing issues per site.

Facets module can't transform its facet links into checkboxes because of a missing js- class.

This is a good example. Let's look deeper.

If there was no stripping-out of classes - the functionality will not work (there is no component in CivicTheme, for example), a developer would dive into code and realise that some work needs to be done to add a support for this (new component etc.), but the site could be affected because a site builder could have accidentally misconfigured the contrib leading to "bleeding" styles.

If there is a stripping-out of classes in place (what we have now) - the functionality will not work, a developer would dive into code and realise that some work needs to be done to add a support for this (new component etc.), but the site will not be affected in any way, so there are no surprises on pages that may or may not be affected by this contrib. If the classes are really harmless for the front-end, a developer can use $variables['modifier_class'] = FALSE; to "opt-out" from stripping-out the classes.

So, it comes down to developer being surprised that a contrib does not work out-of-the-box (and needed to debug why) vs having a really solid front-end that would be affected only when a developer consciously enables the original classes.

Developer's surprise can be avoided if they read documentation (which may or may not exist at the moment of writing this, but it is clearly a fixed-scope item to write such documentation).

The worst impact is standard Form API losing all meaningful and valid attributes that have nothing to do with styling:

This looks like a defect. 1.8 has brought Styleguide and testing of form elements, including testing of passing of attributes.
The attributes passing itself was refactored to handle more cases. Maybe this is not an issue anymore.

<!-- BEGIN OUTPUT from 'core/modules/system/templates/input.html.twig' -->
<input my-custom-attributes="abc" data-abc="1" pattern="[A-Za-z]{3}" minlength="10" spellcheck="true" placeholder="Enter a log message" data-drupal-selector="edit-log" type="text" id="edit-log" name="log" value="" size="30" maxlength="128" class="form-text form-element form-element--type-text form-element--api-textfield" tabindex="-1">

The custom attributes and other attributes from for API should be rendered by CivicTheme - if they are not - it is a defect (but see above - we have fixes the attributes in 1.8).

The CSS classes though - they should not be used for anything else but presentation. And CivicTheme provides that presentation using own classes. If a contrib uses CSS to bind JS events or any other custom functionality - they are doing it wrong by occupying the space used purely for presentation. And they should fix it to use attributes rather than classes. The `js-*` is an old-school approach from times when custom attributes were not supported by browsers. It should not be used in the modern development practices.

-----

Next steps:

There is no way that CivicTheme is going to remove the stripping-out of classes altogether - as mentioned above - this may potentially break front-end leading to upset client's and their site visitors.

But we can create a more robust system of opt-in/opt-out and, potentially, a curated list of supported modules whose classes would not need to be stripped-out.

We are open to other suggestions on how to organise this better.

This suggestion adds a new twist to the paradigm of "configuration paragraphs": we already use similar approach for the "Automated list", where the configuration of what would be displayed is passed down the processing pipeline to the view.

I like this suggestion from the point of re-usability of settings between content types: if a new content type wants to use Banner overrides - adding a "Banner override" paragraph should provide such functionality in an easy way with all the "wiring" already in-place.

The scope of the implementation needs to be widen to include possible future overrides for other components added within CivicTheme or in a consumer site.

The paragraph for a specific component can be collapsed or expanded by OOTB CivicTheme based on the component itself to make it more apparent for the content editor what the override fields are, which will make the Banner Overrides UI looking to closer to what it is now than what it is on the screenshot in the description.

This will also require an update hook to migrate existing site to use such configuration paragraphs.

@sime
Thank you for your suggestion. This is solid.

The change looks good, but we cannot simply apply it as-is: we need to allow existing sites to opt-out using a feature flag (we do not want to expose this as a per-component option because the functionality here should be enabled by default).

@sime
Could you please re-upload the images. Thanks

For any sub-component of the `Basic content` UIKit component, there is a CSS override that forces parent component's theme on the descendant components - buttons, heading, paragraphs etc. etc.

Please try applying a button style and switching the `Content` component to Dark to see that these styles apply automatically.

The only reason for the Drupal's WYSIWYG to have `ct-theme-light` in the button (and other components) styles is to provide an editor's styles (CSS included in the WYSIWYG editor) in the admin theme, like Claro, which is presumably uses "light" theme.

So, there is nothing you need to do - the `ct-inherit` class is not needed as it is already implemented and automatically handled.

@sime
Every site owner have their own opinion about what timestamp should be used in what type of a card.
Currently, there are 3 for `Page`:
- Created date
- Updated date
- Manually set Last Updated date

This issue is about implementing such flexible configuration.

@sonnykt
So what component would CivicTheme use for those Facets exactly? CivicTheme is a design system and only works with supported components. Any added classes by 3-rd party modules will be bringing styles that are not 100% compatible with CivicTheme and a developer would need to style them anyway.

@joshua1234511
Could you please follow the steps for "Non LaunchPad" an check if the components are present. Thank you

Addressed in 1.5.3

alex.skrypnyk → created an issue.

Implemented in https://git.drupalcode.org/project/testmode/-/commit/a6fae85a39e57c4525a...

With generated_content, you have full control over what content is being created and how to create it. With Devel Generate it is all completely random. Each module has own use-cases.

Generated Content is used, for example, to generate pseudo-random content to then use in Visual Regression testing where the content must remain the same to get the snapshots look the same.

Generated Content is a harness for custom code, where Devel Generate is more out-of-the-box commands.

Generated Content is not about creating initial content (default_content is very good for that), but rather about having a re-produceable outcome on each build. Very useful when building profiles and other scaffolds that still require some content filling.

Generated Content is used to test CivicTheme: https://default.civictheme.io/generated-content/components

Looks like it is all working. Closing as "works as designed".

Resolved in UIKIt 1.8 and will released in 1.8

Implemented in UIKit 1.8 and will be released in 1.8

Implemented and will be released in 1.8

Implemented in UIKit 1.8 and updated in dev.

Will be released in 1.8

Updated and will be released in 1.8.

Implemented in https://git.drupalcode.org/project/civictheme/-/commit/8f888fcf647007c48...

`layout--single-column` and `layout--single-column-contained` layouts were mapped to new layout `layout--three-columns` and are now deprecated (will be removed in 1.9).

Closing as cannot reproduce

Implemented and will be released in 1.8

There should not be a case when this takes place. However, hiding the error from the end-user makes sense to me.

Noting that we already log the reason why the returned view is NULL to watchdog.

Merged and will be released in 1.8

Merged and will be released in 1.8

This module was not published on Packagist.

It is now published on Packagist under a new namespace. The existing documentation is correct and does not need to be updated.

alex.skrypnyk → created an issue.

alex.skrypnyk → created an issue.

Fixed and will be released in 1.8

alex.skrypnyk → created an issue.

Could you please re-run your steps with the patch to make sure that there is no WSOD and Drupal installs the theme and configurations completely.

Fixed and will be released in 1.8

Fixed and will be released in 1.8

Fixed and will be released in 1.8

@Kristen Pol

CivicTheme does not use JSON API module. Could you please provide an exact error you encountered.

Also, please restart the installation from scratch - if you are encountered WSOD during installation - Drupal did not fully install all the required configuration.

Thank you, @sime

Confirming - this regression was introduced in 1.7

Fixed and will be released in 1.8

@smethawee
This is a php script that you need to run from the `civictheme` directory.

`ddev php` runs a script in the root of the project rather than in the specified directory. We cannot provide support for specific installations - we simply do not know how you have your project configured. Please consult DDEV documentation on how to run scripts from a specific directory.

Please follow the steps specified here https://docs.civictheme.io/development/drupal-theme/sub-theme

The starterkit with a script to update namespaces and machine names is available in CivicTheme starting version 1.0.
If users do not have PHP installed - they can manually replicate what a script does (but this is an error-prone process and we are not recommending it).

Please see https://docs.civictheme.io/development/drupal-theme/sub-theme

@a-saini
You are modifying the files in the base CivicTheme, but these should not be modified because this is a base theme. Please create a sub-theme to provide overrides.

@thisisalistairsaccount
This looks like the same issue realated to the incomplete install. Could you please re-install and apply a patch as described on the main page.

@thisisalistairsaccount

noting outstanding issues with "The "civictheme_one_column" plugin does not exist"

I think this is a source of all of the issues: looks like the installation does not finish successfully resulting in incomplete installation.

Please try to apply a patch for local GovCMS to get the theme installed. If this is indeed an issue related to the missing "civictheme_one_column" plugin - then there is nothing we can do in CivicTheme - it is a core issue.

@thisisalistairsaccount
If you see this occurs - something went very wrong with the list.
For all cases on when the view would be NULL, is a watchdog record created.

Is it possible to provide this watchdog record here?

@abhishek_gupta1
Appreciate the help, but we cannot simply put the missing view - the engine behind the Automated List - into a partial condition and proceed the processing as if everything is okay (as per your MR). I will wait for more information on how to reproduce this issue.

Removed in 1.8 in https://git.drupalcode.org/project/civictheme/-/commit/9300b98c38dc589bd...

Thanks, everyone, for investigation and contributing to the resolution.

Looks like the resolution is to document steps.

I have updated the project page on D.O. → , the generic and GovSMS SaaS-specific installation instructions on the documentation site.

Implemented and will be released in 1.8

Confirming that this is a vlid issue and it will be resolved in 1.8

Confirming that this is a valid case and that it will be tackled in 1.8

alex.skrypnyk → created an issue.

This approach is used by multiple hosting providers. They all have their own ways of injecting variables and they all use their own variable names.

The proposed change would help to align the implementation across providers.

Ok, seeing that the updated MR passed testing and that the test-only PR has failed. Also tested this locally and all looks good.

Marking RTBC.

@larowlan

The test only pipeline isn't working here because there's no changes to *Test.php files so we need to run locally to verify, I'll do that.

This is what I was trying to explain to @smustgrave in Slack, but failed since I do not understand fully how GitLab tests these test-only VS full test (I do understand @smustgrave's point that a test need to be proving that it would test an actual change and really appreciate that he explained all that to me).

It looks like that would not work as per your comment.

So, just going to wait for the HEAD of 11.x to become unblocked.

@smustgrave
Thank you. Really appreciate your help here.

HEAD is failing because of https://www.drupal.org/project/drupal/issues/3417066 📌 Upgrade PHPUnit to 10, drop Symfony PHPUnit-bridge dependency Fixed

Hoping it will be fixed soon.

This change broke relative path resolution

Stopped working (worked in 11-alpha)
./vendor/bin/phpunit -c ./web/core ./web/core/modules/user/tests/src/Unit/UserAccessControlHandlerTest.php

Works
./vendor/bin/phpunit -c ./web/core $(pwd)/web/core/modules/user/tests/src/Unit/UserAccessControlHandlerTest.php

Implemented and will be released in 1.8

@joshua1234511
Thank you for working on this. I've tested the changes and it all works great.

The relevant issue with caching lies within the FocalPoint module, so we will just have to rely on it being accepted there.

Implemented in https://git.drupalcode.org/project/civictheme/-/commit/b44e45ddfd378ee9b...

xdebug will die on large codebases. pcov won't. try running the whole suite with pcov and compare the coverage scope and run speed (should be 10x faster than xdebug)

We looked deeper into the issue and it appears that the CivicTheme incorrectly uses layout functionality.

The layout template is expected to be available site-wide and, therefore, not supposed to contain any other template inclusions. For styling, the layout could reference a library (which in own turn can use CSS to style that layout). The expectation is that every theme is responsible for providing styles to support layouts.

At his point, it looks like we definitely need to simplify the layout template to not include any other components. We also need to look into shipping a standalone layout CSS file.

We also need to assess if it is worth ditching our custom layout in favour of the Drupal core's layouts markup and just add our own CSS (need to assess if that CSS is already provided by core).

Once the above investigations conducted, the results will be posted into this issue.