localgovdrupal / localgov_moderngov

Drupal module for Modern.Gov template page generation.
0 stars 0 forks source link

ModernGov template page generation module #1

Closed Adnan-cds closed 2 years ago

Adnan-cds commented 2 years ago

Second incarnation of the Modern.Gov template page module. I have polished up Eke's original code by:

Recap

To serve as a ModernGov template, a page needs to meet two requirements:

How to test

Remaining issue

While testing, realized that any relative URL sitting inside embedded Javascript (e.g. drupalSettings) won't change to absolute URL. Not sure how big a problem that is.

finnlewis commented 2 years ago

Discussing this in Merge Monday call.

@Adnan-cds points out that inline javascript in the head is not covered by the current approach to search and replace convertion of relative to absolute URLs.

@ekes wonders if there's a way to hook into the libraries attach process to convert urls.

See https://democracy.croydon.gov.uk/uuCoverPage.aspx?bcr=1 for example of ModernGov in action.

Caching is also an issue. For blocks that are cached, with relative links, these need to be rendered differently depending on whether they are on the ModernGov system or on Drupal.

Next steps on this:

markconroy commented 2 years ago

From a frontend/template perspective this looks fine to me.

Once commited/merged, I'll create a corresponding template for the localgov_base theme that adds our specific markup so we can support this out of the box.

Adnan-cds commented 2 years ago

will try the suggestion from @ekes to hook into the libraries.

Here are my thoughts so far: From hook_page_attachments_alter(), we can inspect all attachments. The relevant ones for URL conversion are of two types:

The problem now is, there is no clear pattern for any relative URL within the above two. For example, we may have something like this:

$attachments['drupalSettings']['foo'] = [
  'zero' => '/i/am/a/path',
  'src'   => '/i/am/also/a/path',
]
$attachments['html_head']['bar'] = [
  '#type' => 'html_tag',
  '#tag'   => 'script',
  '#value' => 'var a_path = "/yet/another/path"; do_something(a_path);'
]

As you can see, there is no easy way to identify who is a relative path and who is not unless you are looking for an exact string pattern such as 'a_path = "/yet/another/path"' which is easy for a site specific custom module, but not in a contrib module.

Any hints are welcome. @finnlewis @ekes @andybroomfield @markconroy

finnlewis commented 2 years ago

Notes from Merge Monday today:

Happy to push ahead with this and document the limitations in the README. (Already noted here https://github.com/localgovdrupal/localgov_moderngov/blob/1.x/README.md#good-to-know)

Adnan-cds commented 2 years ago

Thanks for all the approvals. I am merging...