Thuwal
Account created on 30 December 2020, over 3 years ago
#

Merge Requests

More

Recent comments

๐Ÿ‡ธ๐Ÿ‡ฆSaudi Arabia martins.bruvelis Thuwal

martins.bruvelis โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ธ๐Ÿ‡ฆSaudi Arabia martins.bruvelis Thuwal

martins.bruvelis โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ธ๐Ÿ‡ฆSaudi Arabia martins.bruvelis Thuwal

I can confirm that adding the patch 'guzzle-description-loader-fix.patch' usign composer.json, to arthurkirkosa/guzzle-description-loader module, resolves the issue:

,
            "arthurkirkosa/guzzle-description-loader": {
                "3406108 - guzzle-description-loader dependency is broken": "https://www.drupal.org/files/issues/2023-12-05/guzzle-description-loader-fix.patch"
            }
๐Ÿ‡ธ๐Ÿ‡ฆSaudi Arabia martins.bruvelis Thuwal

martins.bruvelis โ†’ created an issue.

๐Ÿ‡ธ๐Ÿ‡ฆSaudi Arabia martins.bruvelis Thuwal

martins.bruvelis โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ธ๐Ÿ‡ฆSaudi Arabia martins.bruvelis Thuwal

martins.bruvelis โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ธ๐Ÿ‡ฆSaudi Arabia martins.bruvelis Thuwal

martins.bruvelis โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ธ๐Ÿ‡ฆSaudi Arabia martins.bruvelis Thuwal

martins.bruvelis โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ธ๐Ÿ‡ฆSaudi Arabia martins.bruvelis Thuwal

martins.bruvelis โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ธ๐Ÿ‡ฆSaudi Arabia martins.bruvelis Thuwal

martins.bruvelis โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ธ๐Ÿ‡ฆSaudi Arabia martins.bruvelis Thuwal

martins.bruvelis โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ธ๐Ÿ‡ฆSaudi Arabia martins.bruvelis Thuwal

> Comment #9 โœจ Provide option to add External Site URL Needs work
>

Addind a link in a menu towards this external site doesn't do the job ?

Indeed, adding extra links to non-Drupal sites doesn't do the job. Why do organizations choose to have multiple sites instead of having a single website?

  • Out of 23 related department/organiztaional unit mentions on https://profiles.stanford.edu/jelena-vuckovic profile page, only six have added links to the respective website front pages. Why has it been so hard to add links to the related websites? If all related websites were part of Drupal CMS, then Micro Sites entities would provide an easy and consistent UX for and related sites.
  • Website content items may relate to Drupal CMS and non-Drupal CMS sites hosted by the same organization. The sites typically represent departments or specific business functions (e.g., administrative organization units), or one-off or repeating activities (e.g., a website for a yearly conference or one-off workshop). How can such relationships be managed in a consistent standard approach?
  • For example, if an event (created as a dedicated Node bundle content type for events) has affiliation to Department A (using Drupal Micro Site), Department B (using non-drupal site), and Department C (using Drupal Micro Site). How would you build a consistent UX to specify department affiliations and and each affiliated department? A simple link field does not provide a reliable, easy-to-use way to manage affiliations. Instead, there would have to be a custom entity type that creates and stores entities for each department and website within an organization. When Micro Site entities already make up the majority of websites within an organization, why create another custom entity type in addition to Micro Site?

Profile page example, where over 50% of related department/organizational unit websites are missing links.

๐Ÿ‡ธ๐Ÿ‡ฆSaudi Arabia martins.bruvelis Thuwal

Comment #5 โœจ Provide option to add External Site URL Needs work

Another simpler way to look at a Micro Site entity is by comparing it to core Drupal Link Fields that provide "Allowed link type" options:

  • Internal links only
  • External links only
  • Both internal and external links

Currently, Micro Site only provides the Internal links only option (i.e., publish and link to Micro Site entities that are hosted on Drupal CMS) and excludes the ability to provide Both internal and external links (i.e., link to Micro Site entities that are not hosted on Drupal CMS). From the business logic, to avoid accidentally publishing content primarily on a Micro Site entity that links to a non-Drupal website, it would be necessary to exclude the non-Drupal website Micro Site entities from autocompletion of the primary "site_id" field.

๐Ÿ‡ธ๐Ÿ‡ฆSaudi Arabia martins.bruvelis Thuwal

If you have an external site of a suborganisation, then why create a micro site for this external site?

Universities, Societies, and Government institutions, etc., typically have tens to hundreds of various departments (sub-organizational units) responsible for specific areas of business functions or representing various geographic office locations.

> sounds like a very specific use case.
It is more common than it might seem. For example, 71% of the top 100 Universities use Drupal. In such cases, each "Micro Site" can represent not only a publicly hosted website but also an affiliation. It is essential to provide navigational links to respective affiliations.

Most medium to large organizations have physical offices across various local or global regions. Each location typically represents a branch of a larger organization. Then news, events, people profiles, pages, etc. have a clear content grouping with one primary Micro Site and with one or more secondary affiliated Micro Sites.

Currently, Micro Site allows publishing and linking content (Nodes) only to such affiliations that use the Drupal CMS with the Micro Site module and leaving all other non-Drupal websites from the same institution/organization inaccessible from the Drupal CMS with the Micro Site.

As such, adding the option to provide a link to non-Drupal CMS website for a Micro Site entity would address one of the key reasons why organizations opt for a multi-site solution, without enforcing all of the existing organization websites to migrate to the Druplal CMS and not requiring some other solution such as manual linking to non-Drupal CMS related websites from the same ogranization.

๐Ÿ‡ธ๐Ÿ‡ฆSaudi Arabia martins.bruvelis Thuwal

Resolved in the latest drupal/core by issue ListItemBase::generateSampleValue() white screen when options list is empty (3331397) ๐Ÿ› ListItemBase::generateSampleValue() white screen when options list is empty Fixed

๐Ÿ‡ธ๐Ÿ‡ฆSaudi Arabia martins.bruvelis Thuwal

It seems that the item.label in with the addition of id in the label doesn't get parsed correctly when the label contains special characters, such as "comma" an "double quotes". Using this.value as the label, seems to solve the issue. Additionally, the (id) can be removed from being displayed to the end users by removing the id inside the brackets at the end of the string in assets/js/autocomplete_delux.js by replacing:

    this.element = $(
      '<span class="autocomplete-deluxe-item">' + item.label + "</span>"
    );

with:

    // Use value as label because of the double quote and comma character encoding.
    // Then remove the entity id in bracketsfrom the end of string.
    const noLabelId = this.value.replace(/[ ]\([^()]+\)(?=[^()]*$)/gi, "");
    this.element = $(
      '<span class="autocomplete-deluxe-item">' + noLabelId + "</span>"
    );
๐Ÿ‡ธ๐Ÿ‡ฆSaudi Arabia martins.bruvelis Thuwal

@Anybody โ†’ , regarding,

Should we also add a line to the status report explaining this option and showing its status? I think that wouldn't be too bad?

Indeed, it would have been very helpful to see a warning displayed after a Bootstrap5 theme update that added field.html.twig template, resulting in Fences module not working. However, I am not sure how to reliably check if currently enabled non-core theme has a field.html.twig template to show a warning in the status report that an issue is detected. Also, I have not checked if there are any contributed themes, that would not work with Fences module. When I checked if Fences module is working Bartik theme, it looked that Fences is working, contrary to what is stated in the issue description above (maybe the issues with Bartik theme was resolved with one of the other issues patches).

Considering that Fences module checks for field.html.twig template via code strpos($theme_registry['field']['path'], 'core') === 0 it seems that Fences module will only override the field.html.twig template and not any of the more specific field templates, such as, field--entity-reference.html.twig. This seems to allow to override specific fields or group of fields via the respective field--*.html.twig templates, while allowing the base field field.html.twig template to be overridden by Fences module.

Regarding,

What we'll definitely need here are tests. For the option disabled and enable to ensure the expected functionality. Could you add them in the next step please?

  1. For the disabled option, all of the existing tests are already sufficient, it should behave exactly as before.
  2. For the enable option, there are following potential test cases to consider:
  • if the theme is from core, nothing should change, and all existing tests are sufficient
  • if the theme is not from core, then all of the tests could be re-run with some non-core theme, and test input/outputs should produce the same code as with core theme.

From the above, the biggest issue is that existing tests do not check that the Fences module would stop working if contributed non-core theme is used. Most likely, because it is not easy to add a dependency on some contributed non-core theme, that could potentially add/remove field twig template from the theme with any of the theme updates. For example, in a recent Bootstrap5 theme update, the field.html.twig template was added, and Fences module stopped working without providing any notification, warning or error messages.

Therefore, could someone clarify if it is a common practice to add test where it is required to install, enable, and check functionality with a contributed non-core theme? If it is not common practice then we could proceed without adding any additional tests, as existing tests arealdy check that it works as expected.

๐Ÿ‡ธ๐Ÿ‡ฆSaudi Arabia martins.bruvelis Thuwal

I added a configuration option to allow the override of the field template only if it came from the core or allow to override of the field template for all themes.

@Anybody โ†’ and @thomas.frobieter โ†’ would adding Fences module setting to allow to override of the field template for all themes with the default option set to only allowing core theme override avoid a breaking change?

๐Ÿ‡ธ๐Ÿ‡ฆSaudi Arabia martins.bruvelis Thuwal

I can confirm that running drush updatedb does not work when updating from 3.0@RC to 3.0.2 and results in a warning and error:

>  [warning] Post update function bootstrap5_post_update_install_stable9 not found in file bootstrap5.post_update.php
>  [error]  Update failed: bootstrap5_post_update_install_stable9

.

However, running updates from the Drupal Admin menu in the browser works without any warnings or errors (same as it was reported by @chriscant ).

Clearing caches does not help to resolve the issue with drush updatedb.

๐Ÿ‡ธ๐Ÿ‡ฆSaudi Arabia martins.bruvelis Thuwal

Also, when enabling the setting "No wrapping HTML for entity content", it is necessary to use the module Manage display โ†’ to resolve the bug reported in the issue
Unable to remove "Author" and "Created" data from Entity Reference fields. โœจ Unable to remove "Author" and "Created" data from Entity Reference fields. Active The module Manage display โ†’ allows to correctly hide/display all of the fiends, including, "Author" and "Created" fields.

๐Ÿ‡ธ๐Ÿ‡ฆSaudi Arabia martins.bruvelis Thuwal

@Anybody, regarding

check why existing tests fail. This should NOT be a breaking change (BC).

Drupal 10 no longer has "stable" theme, replaced it with "stark" theme, resolved the tests failure.

add test coverage for the new functionality to proof this works as expected

OK, added a test to check if the default "stark" theme content wrapper element (article) is removed, when enabling the new setting.

Improve description to understand what this does.

Please, check the revised description.

Production build 0.69.0 2024