-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
base: develop
Are you sure you want to change the base?
Templatable document title #28979
Changes from 4 commits
637af1d
38e31ad
f99de0b
c4138a3
2ad3656
ee03527
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
/* | ||
Copyright 2025 New Vector Ltd. | ||
|
||
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial | ||
Please see LICENSE files in the repository root for full details. | ||
*/ | ||
|
||
import { expect, test } from "../../element-web-test"; | ||
|
||
/* | ||
* Tests for branding configuration | ||
**/ | ||
|
||
test.describe('Test without branding config', () => { | ||
test("Shows standard branding when showing the home page", async ({ pageWithCredentials: page }) => { | ||
await page.goto("/"); | ||
await page.waitForSelector(".mx_MatrixChat", { timeout: 30000 }); | ||
expect(page.title()).toEqual('Element *'); | ||
}); | ||
test("Shows standard branding when showing a room", async ({ app, pageWithCredentials: page }) => { | ||
await app.client.createRoom({ name: "Test Room" }); | ||
await app.viewRoomByName("Test Room"); | ||
expect(page.title()).toEqual('Element * | Test Room'); | ||
}); | ||
}); | ||
|
||
test.describe('Test with custom branding', () => { | ||
test.use({ config: { | ||
brand: 'TestBrand', | ||
branding: { | ||
title_template: 'TestingApp $ignoredParameter $brand $status $ignoredParameter', | ||
title_template_in_room: 'TestingApp $brand $status $room_name $ignoredParameter' | ||
} | ||
}}); | ||
test("Shows custom branding when showing the home page", async ({ pageWithCredentials: page }) => { | ||
await page.goto("/"); | ||
await page.waitForSelector(".mx_MatrixChat", { timeout: 30000 }); | ||
expect(page.title()).toEqual('TestingApp TestBrand * $ignoredParameter'); | ||
}); | ||
test("Shows custom branding when showing a room", async ({ app, pageWithCredentials: page }) => { | ||
await app.client.createRoom({ name: "Test Room" }); | ||
await app.viewRoomByName("Test Room"); | ||
expect(page.title()).toEqual('TestingApp TestBrand * Test Room $ignoredParameter'); | ||
}); | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
title_template_in_room?: string; | ||
}; | ||
|
||
force_verification?: boolean; // if true, users must verify new logins | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -228,6 +228,9 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> { | |
private subTitleStatus: string; | ||
private prevWindowWidth: number; | ||
|
||
private readonly titleTemplate: string; | ||
private readonly titleTemplateInRoom: string; | ||
|
||
private readonly loggedInView = createRef<LoggedInViewType>(); | ||
private dispatcherRef?: string; | ||
private themeWatcher?: ThemeWatcher; | ||
|
@@ -281,6 +284,9 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> { | |
// object field used for tracking the status info appended to the title tag. | ||
// we don't do it as react state as i'm scared about triggering needless react refreshes. | ||
this.subTitleStatus = ""; | ||
|
||
this.titleTemplate = props.config.branding?.title_template ?? '$brand $status'; | ||
this.titleTemplateInRoom = props.config.branding?.title_template_in_room ?? '$brand $status | $room_name'; | ||
} | ||
|
||
/** | ||
|
@@ -1106,6 +1112,7 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> { | |
} | ||
this.setStateForNewView({ | ||
view: Views.WELCOME, | ||
currentRoomId: null, | ||
}); | ||
this.notifyNewScreen("welcome"); | ||
ThemeController.isLogin = true; | ||
|
@@ -1115,6 +1122,7 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> { | |
private viewLogin(otherState?: any): void { | ||
this.setStateForNewView({ | ||
view: Views.LOGIN, | ||
currentRoomId: null, | ||
...otherState, | ||
}); | ||
this.notifyNewScreen("login"); | ||
|
@@ -1949,21 +1957,32 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> { | |
}); | ||
} | ||
|
||
private setPageSubtitle(subtitle = ""): void { | ||
private setPageSubtitle(): void { | ||
const params: { | ||
$brand: string; | ||
$status: string; | ||
$room_name: string|undefined; | ||
} = { | ||
$brand: SdkConfig.get().brand, | ||
$status: this.subTitleStatus, | ||
$room_name: undefined, | ||
}; | ||
|
||
if (this.state.currentRoomId) { | ||
const client = MatrixClientPeg.get(); | ||
const room = client?.getRoom(this.state.currentRoomId); | ||
if (room) { | ||
subtitle = `${this.subTitleStatus} | ${room.name} ${subtitle}`; | ||
params.$room_name = room.name; | ||
} | ||
} else { | ||
subtitle = `${this.subTitleStatus} ${subtitle}`; | ||
} | ||
|
||
const titleTemplate = params.$room_name ? this.titleTemplateInRoom : this.titleTemplate; | ||
|
||
const title = `${SdkConfig.get().brand} ${subtitle}`; | ||
const title = Object.entries(params).reduce( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
(title: string, [key, value]) => title.replaceAll(key, (value ?? '').replaceAll('$', '$_DLR$')), titleTemplate); | ||
|
||
if (document.title !== title) { | ||
document.title = title; | ||
document.title = title.replaceAll('$_DLR$', '$'); | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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