Skip to content

Conversation

@kjansen2608
Copy link

@kjansen2608 kjansen2608 commented May 13, 2025

What has been changed?

ILIAS system mails previously used a basic HTML template that included the default ILIAS logo and the installation title "Open Source eLearning".
Customizing this mail layout required manually overwriting the corresponding template file in the System-Style, which was technically functional but not aligned with system-wide design principles.

We developed a solution that:

  • Uses a modernized look & feel for system emails
  • Integrates logo and color values from active System-Style
  • Eliminates the need to overwrite the mail template separately
  • Introduces a new UI layout component ("Mail Page") to the kitchen sink for reusability and consistency

Why was it implemented this way?

The main goal was to centralize styling logic and make mail templates easier to manage when applying individual System-Stylings. By providing a new SCSS file which can be customized via System-Styles , we aimed to:

  • Ensure visual consistency across interfaces
  • Reduce the need for manual adjustmenting Templates of the System-Style

This decision was based on discussions we had with @Amstutz, where he gave us the initial impulse to bring the mail layout into the UI framework.

A technical challenge we encountered was embedding the logo.
To ensure that the logo would be delivered as an attachment and correctly referenced in all email clients, we had to directly embed the image from a given file path.
This method gives full control over how the image is embedded and allows the mail to include it as an actual attachment, increasing compatibility and flexibility for users.


How was it tested?

Manual testing was conducted across a wide range of mail clients to ensure rendering consistency.
This was particularly important because many email clients use outdated rendering engines, leading to inconsistent support for modern HTML and CSS standards.
Not all clients could be tested due to device constraints.
Results:

Mail Client Tested on Known Issues
Apple Mail iOS ✅ 04.10.2024
Apple Mail macOS ✅ 04.10.2024
Gmail Desktop -
Gmail iOS ✅ 22.10.2024
Gmail Android ✅ 15.10.2024
Gmail Mobile Webmail -
Outlook Windows -
Outlook Windows Mail -
Outlook macOS ✅ 04.10.2024
Outlook.com ✅ 15.10.2024
Outlook iOS ✅ 04.10.2024
Outlook Android ✅ 15.10.2024
Yahoo! Desktop -
Yahoo! iOS -
Yahoo! Android -
AOL Desktop Webmail -
AOL iOS -
AOL Android -
Samsung E-Mail Android -
Thunderbird Windows ✅ 04.10.2024
Thunderbird macOS -
ProtonMail Desktop Webmail ✅ 04.10.2024
ProtonMail iOS ✅ 22.10.2024
ProtonMail Android ⚠️ 04.10.2024 Mail scrolls horizontally
WEB.DE Desktop Webmail ✅ 15.10.2024
WEB.DE iOS ✅ 16.10.2024
WEB.DE Android -

Notes for reviewers

We're still looking for a clean and user-friendly way to notify developers and designers of System-Styles that two SCSS files need to be compiled now instead of just one.

Currently, this introduces a second compilation layer, which is not ideal.
An alternative approach, extracting the mail section from the compiled CSS, was discussed in the same meeting with @Amstutz, but was ultimately dismissed by mutual agreement, as it was considered less sustainable in the long term.

We're considering developing a small script to assist with this step. Feedback on this point is welcome :)


Related Tickets / References

Feature Request:

https://docu.ilias.de/go/wiki/wpage_7280_1357
Due to the work on the UI component and the PR, the FeatureRequest will be reworked accordingly before it is presented again to the JourFixe for 11/Trunk.

Internal Reviews:

Related PR's:

@kjansen2608 kjansen2608 changed the title Default-HTML-Frame for Mail in System Style Default-HTML-Frame for Mail in System Style (1/3) May 13, 2025
@marvin-matuschek marvin-matuschek force-pushed the mail-component branch 2 times, most recently from c230e6a to 3fe04a4 Compare May 13, 2025 12:25
@Amstutz Amstutz assigned yvseiler and unassigned Amstutz Aug 28, 2025
@thibsy
Copy link
Contributor

thibsy commented Oct 23, 2025

Hi all,

We apologise for the delay here. We want to get this included in ILIAS 11, so we try to get the full review (interface and implementation) done until Monday next week, so we can also present this at the last JF before release.

Please tell us if you already know you will not be able to implement requested changes until the end of next week, then we will prioritise other PRs.

Kind regards
@klees, @yvseiler and @thibsy

@mjansenDatabay
Copy link
Contributor

Hi @thibsy,

thanks a lot for your efforts. As far as I know, the deadline is Tuesday, 28 October 2025.
I am confident that my colleagues at Databay will be able to invest some time on Tuesday (after the JF discussion) to address the changes requested by the UI coordinators, of course, depending on the scope and complexity of those changes.

Best regards,
Michael

Copy link
Contributor

@klees klees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @kjansen2608,

Thanks a lot for your contribution to the UI components!

Please note that I will try to abbreviate FQDN's and refer to the public interface as C and implementation as I

About the concrete changes, please answer the following questions:

  • HTML and DOCTYPE tags: since this template is now part of the UI framework, it would probably be good for us to know, what these definitions do exactly. Can you explain, why i.e. a simple <!DOCTYPE html> is not enough, and why both of these tags need special attributes?

Please consider the following suggestions. You do not need to follow them, but please indicate shortly why you prefer to do otherwise:

  • C\Layout\Page::withFooterURL: you are more familiar with the mail use-cases than I am, so I leave this up to you: I think we leave many choices up to the consumer if we accept a link UI component here. Wouldn't it i.e. be better if either all or none of the footer URLs open in a new viewport? For consistency I suggest to accept an ILIAS\Data\Link link here, so we can keep this universal and an implementation detail.
  • Logo-source: you convert the image into a base-64 string in your example. I assumed you can also provide an actual file path, is this not the case? Could you elaborate why this would be different to the other cases like the stylesheet, where you accept a path? If there are no good reasons for the difference, it would be preferable to use a path for both.

Please implement the following changes:

  • Correctness on construction: we follow this principle most of the time. The C\Page\Standard was designed with many mutators, because of its orchestration via ILIAS\GlobalScreen. I don't think we need this kind of flexibility and complexity with the C\Page\Mail component. Creating a mail, IMO, should not happen across many ILIAS components or layers. Please make all (mandatory, IMO all except footer-URL?) arguments in C\Page\Factory::mail() non-nullable, keep the others as is but order them correctly. Strip down the C\Page\Mail interface completely, remove all mutators and has* methods entirely (interface and implementation), only keep getters inside the implementation.
  • Stylesheet validity: please throw an \InvalidArgumentException during construction already if the file cannot be found, instead of in the renderer.
  • ilUIFramework usage: this class does not exist anymore. Please check why this was necessary and remove it, if there is no need anymore.
  • I\Page\Renderer::render(): please check the instanceof the implementation, not the interface. You can read why this is important here
  • tpl.mailpage.html I: you declare the xmlns attribute on the <html> tag twice, I assume this is not intentional. Please remove it.
  • tpl.mailpage.html II: you declare the <head> tag twice, I assume this is not intentional. Please remove it.
  • I\Page\Mail: please make the private properties protected to leave them open for extension.

Kind regards,
@thibsy (and agreed with @klees)

@klees
Copy link
Contributor

klees commented Oct 24, 2025

☝️ I have posted this, but @thibsy did the most of the work. Thanks!


namespace ILIAS\UI\Implementation\Component\Layout\Page;

use http\Exception\RuntimeException;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks wrong here.

@kjansen2608
Copy link
Author

Hi @thibsy, @klees,

thank you for your review.

  • HTML and DOCTYPE tags: since this template is now part of the UI framework, it would probably be good for us to know, what these definitions do exactly. Can you explain, why i.e. a simple <!DOCTYPE html> is not enough, and why both of these tags need special attributes?

Yes HTML 5 would be a better fit - we just took it from the old template. Due to limited time before the release, we won't be able to test it beforehand with all the email clients we listed above in the PR description. But we don't expect any issues with it. We’d be happy to include it in our roadmap and perform the testing afterwards.

  • C\Layout\Page::withFooterURL: you are more familiar with the mail use-cases than I am, so I leave this up to you: I think we leave many choices up to the consumer if we accept a link UI component here. Wouldn't it i.e. be better if either all or none of the footer URLs open in a new viewport? For consistency I suggest to accept an ILIAS\Data\Link link here, so we can keep this universal and an implementation detail.

done

  • Logo-source: you convert the image into a base-64 string in your example. I assumed you can also provide an actual file path, is this not the case? Could you elaborate why this would be different to the other cases like the stylesheet, where you accept a path? If there are no good reasons for the difference, it would be preferable to use a path for both.

We cannot use a file path because they are not supported in every email client. Even the stylesheet is not using a file path it is included with file_get_contents

  • Correctness on construction: we follow this principle most of the time. The C\Page\Standard was designed with many mutators, because of its orchestration via ILIAS\GlobalScreen. I don't think we need this kind of flexibility and complexity with the C\Page\Mail component. Creating a mail, IMO, should not happen across many ILIAS components or layers. Please make all (mandatory, IMO all except footer-URL?) arguments in C\Page\Factory::mail() non-nullable, keep the others as is but order them correctly. Strip down the C\Page\Mail interface completely, remove all mutators and has* methods entirely (interface and implementation), only keep getters inside the implementation.

done

  • Stylesheet validity: please throw an \InvalidArgumentException during construction already if the file cannot be found, instead of in the renderer.

done

  • ilUIFramework usage: this class does not exist anymore. Please check why this was necessary and remove it, if there is no need anymore.

done. This was a mistake because of a rebase

  • I\Page\Renderer::render(): please check the instanceof the implementation, not the interface. You can read why this is important here

done

  • tpl.mailpage.html I: you declare the xmlns attribute on the <html> tag twice, I assume this is not intentional. Please remove it.

done

  • tpl.mailpage.html II: you declare the <head> tag twice, I assume this is not intentional. Please remove it.

done

  • I\Page\Mail: please make the private properties protected to leave them open for extension.

done

Kind regards,
@lscharmer, @marvin-matuschek & Kelly

Copy link
Contributor

@thibsy thibsy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @kjansen2608,

Thx for implementing the changes so quickly.

I have found some nit picks which I have commented inline. In addition, please:

  • Logo-src: we understand that assets cannot be included, but it makes the lives of our consumers much easier if they can provide asset paths and we will do the work. Therefore, please implement the "logo src" similar to the stylesheet (validation of path inside constructor, conversion in renderer) and rephrase the according variable-, property-, and method-names to match "logo path" as well. If consumers are expected to provide specific mime-type(s), i.e. for compatibility with base64_encode(), also add a usage rule for this to the factory description.

Afterwards we can merge this.

Thx and kind regards,
@thibsy

@lscharmer
Copy link
Contributor

Hi @thibsy,

  • Logo-src: we understand that assets cannot be included, but it makes the lives of our consumers much easier if they can provide asset paths and we will do the work. Therefore, please implement the "logo src" similar to the stylesheet (validation of path inside constructor, conversion in renderer) and rephrase the according variable-, property-, and method-names to match "logo path" as well. If consumers are expected to provide specific mime-type(s), i.e. for compatibility with base64_encode(), also add a usage rule for this to the factory description.

sorry for the confusion, this is already possible (and intended) with the current solution, only the examples use a base64 data URL, in the mail implementation PR #9566 this is implemented with a path to an attached image (cid).

One just needs to be careful as the stylesheet path must be embedded, so this expects a file path pointing to a file on the server but the logo does not need to be embedded, so it accepts a file path pointing to the attachments or a data URL.

Best regards
@marvin-matuschek & @lscharmer

@thibsy
Copy link
Contributor

thibsy commented Oct 28, 2025

@lscharmer, I will paraphrase to make sure we understand this correctly: you intentionally leave the possibility to embed a logo from a source which is NOT on the ILIAS server. This must mean, you have a use-case for this in mind? From our POV this currently makes not much sense, maybe we can talk about this briefly on Discord to save time?

adds mail component

adjust docu and move css

adds delos.css to mail template

display logo inline

changes html line-breaks

adds Test for MailPage

uses default header-icon instead of new .jpg

fix test

removes URL from template

updates mail-docu and renames tpl-var

fix header css

design fixes

Fix CS for Mail-Interface

Fix CS for Mail Implementation

remove unrelated fixes

Fix CS for Mail test

Fix CS for Renderer

Fix CS

change ui-mail content type from string to Content

use mail.css instead off delos.css

build delos.css

fix tests

fix docu

use Dataprovider

remove testGetContent

removes usage of call_user_func and add commas

adjust docu

improve mail renderer

apply all suggestions

fix tests

removes mail.css
@lscharmer
Copy link
Contributor

Hi @thibsy,
we have now implemented the changes we discussed via discord:

  • logoSrc is renamed to URL
  • In the data factory it is clarified that only data and cid URL's can be passed as the
    $logo_url, with references to corresponding specifications
  • The Mail Page throws an InvalidArgumentException if the given URL doesn't begin with data: or cid:

Best regards and thanks again for your time :)
@marvin-matuschek & @lscharmer

Copy link
Contributor

@thibsy thibsy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx @lscharmer for your explanation. Changes LGTM now!

@thibsy thibsy merged commit 77c3375 into ILIAS-eLearning:trunk Oct 28, 2025
5 checks passed
@matthiaskunkel
Copy link
Member

Jour Fixe, 10 NOV 2025: Yvonne notified us about changes in the public interface through this PR. Thanks a lot for this very valuable improvement. Changes were already committed to trunk before release 11 branch has been created and are therefore available in ILIAS 11, too.

@marvin-matuschek marvin-matuschek deleted the mail-component branch January 20, 2026 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants