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.

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

Templatable document title #28979

Draft
wants to merge 6 commits into
base: develop
Choose a base branch
from
Draft

Templatable document title #28979

wants to merge 6 commits into from

Conversation

Half-Shot
Copy link
Member

This adds the ability to set a title template via the branding configuration, allowing vendors to customize the document title depending on their needs.

Checklist

  • Tests written for new code (and old code if feasible).
  • [N/A] New or updated public/exported symbols have accurate TSDoc documentation.
  • Linter and other CI checks pass.
  • I have licensed the changes to Element by completing the Contributor License Agreement (CLA)

@Half-Shot Half-Shot requested a review from a team as a code owner January 13, 2025 13:13
4. `title_template`: A template string that can be used to configure the title of the application when not viewing a room.
5. `title_template_in_room`: A template string that can be used to configure the title of the application when viewing a room

#### `title_template` vars
Copy link
Member Author

Choose a reason for hiding this comment

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

There is an issue here that this (by omission) assures these values to be stable. Should we add a disclaimer that these parameters may change at any time?

Copy link
Member

@t3chguy t3chguy Jan 13, 2025

Choose a reason for hiding this comment

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

Because our config flags are to be considered stable and changes should include backwards compatibility, its one of the reason config options tend to need product approval as its something we're committing to supporting for a good long time given we're not semver compliant it is much harder to spot "breaking" configuration changes

Like we still support camelCase & snake_case even though we switched to the latter multiple years ago. Like we still support default_hs_url even though we switched to well-knowns many years ago.

Copy link
Member Author

Choose a reason for hiding this comment

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

Cool. I'll put this under product review then. My understanding is the feature itself was greenlit, though I think we should be aware of the configuration implications.

Copy link
Member

Choose a reason for hiding this comment

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

aware of the configuration implications.

Maintenance implications*

It may want to be an Element Module to avoid that, or Product should derive the exact functions it should support so it can live & be supported mainline


const title = `${SdkConfig.get().brand} ${subtitle}`;
const title = Object.entries(params).reduce(
Copy link
Member Author

Choose a reason for hiding this comment

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

⚠️ I wrote my own templating function here.

I feel like an app this size must be using templating elsewhere but couldn't find any examples or libraries I was particularly keen on, but pointers in this direction would be appreciated.

@@ -50,6 +50,8 @@ export interface IConfigOptions {
welcome_background_url?: string | string[]; // chosen at random if array
auth_header_logo_url?: string;
auth_footer_links?: { text: string; url: string }[];
title_template?: string;
Copy link
Member Author

Choose a reason for hiding this comment

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

Branding seems to consist of mostly urls, but it felt like a reasonable place to put this. Shout if this should be somewhere else.

@Half-Shot Half-Shot added the X-Needs-Product More input needed from the Product team label Jan 13, 2025
@Half-Shot Half-Shot marked this pull request as draft January 13, 2025 15:53
@Half-Shot
Copy link
Member Author

Requires input from @daniellekirkwood to proceed so pausing for the moment.

@t3chguy
Copy link
Member

t3chguy commented Jan 13, 2025

If this does go ahead we'd also need to consider i18n, as currently stands it does not support i18n

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Enhancement X-Needs-Product More input needed from the Product team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants