Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Avoid hard-coded module path in template #35

Closed
donquixote opened this issue Oct 22, 2024 · 2 comments · Fixed by #36
Closed

Avoid hard-coded module path in template #35

donquixote opened this issue Oct 22, 2024 · 2 comments · Fixed by #36
Milestone
0.9.0-beta5

Comments

@donquixote
Copy link
Collaborator

The templates/collabora-online-full.html.twig template contains hard-coded paths in the urls to js and css files.

  <head>
    <script src="/modules/contrib/collabora_online/js/cool.js"></script>
    <link href="/modules/contrib/collabora_online/css/cool.css" rel="stylesheet" />
  </head>

This assumes that the module is in /modules/contrib/collabora_online, which is not guaranteed.
It also assumes that the base url is just /, which means Drupal is not installed in a subdirectory.

Instead, these urls should be composed from variables passed into the template.
@donquixote
Copy link
Collaborator Author

Also interesting: We are adding the library, via '#attached', but it does not really work, because we are not using a regular page html template that would be populated with the assets.

There is an example in Drupal core which uses a placeholder mechanism for assets in an iframe content.
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/media/src/Controller/OEmbedIframeController.php?ref_type=heads#L176
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/media/templates/media-oembed-iframe.html.twig?ref_type=heads

This same technique is used in the main html.html.twig which is used on regular pages.
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/system/templates/html.html.twig?ref_type=heads#L31

We could use the same technique, but for now I would prefer to do something simpler, and simply pass the module path as a variable.

@hfiguiere
Copy link
Collaborator

I was having trouble with making it work with the library so I took that bad shortcut, and kinda forgot about it, even after fixing it once.

donquixote added a commit to donquixote/collabora-drupal that referenced this issue Oct 22, 2024
@donquixote
Issue CollaboraOnline#35: The 'path' part is not needed in hook_theme().
ee32caa
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Oct 22, 2024
@donquixote
Issue CollaboraOnline#35: Pass correct module path to the template.
06ad651
@donquixote donquixote mentioned this issue Oct 22, 2024
Merged
@hfiguiere hfiguiere added this to the 0.9.0-beta5 milestone Oct 22, 2024
@donquixote donquixote mentioned this issue Oct 24, 2024
Merged
donquixote added a commit to donquixote/collabora-drupal that referenced this issue Oct 24, 2024
@donquixote
Merge pull request #2 from donquixote/issue-35-module-path-in-template
71f28e5 
Issue CollaboraOnline#35: Avoid hard-coded module path in template.
hfiguiere pushed a commit that referenced this issue Oct 24, 2024
@donquixote @hfiguiere
Issue #35: The 'path' part is not needed in hook_theme().
1d2065a
hfiguiere pushed a commit that referenced this issue Oct 24, 2024
@donquixote @hfiguiere
Issue #35: Pass correct module path to the template.
150cade
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.9.0-beta5
Development

Successfully merging a pull request may close this issue.

Issue #35: Avoid hard-coded module path in template. donquixote/collabora-drupal
2 participants
@hfiguiere @donquixote