From a345eb42c2b4539df93b4fe0448d4731d9357be8 Mon Sep 17 00:00:00 2001 From: Tim Down <158919+timdown@users.noreply.github.com> Date: Thu, 11 Apr 2024 12:01:27 +0100 Subject: [PATCH] Merge pull request #17712 from overleaf/td-bs5-feature-flag Add feature flag check and per-page opt-in for Bootstrap 5 GitOrigin-RevId: 111474c19f0202efc4e701eef597c7653f8e8b61 --- .../app/src/Features/User/UserPagesController.js | 4 ++++ .../web/app/src/infrastructure/ExpressLocals.js | 15 +++++---------- services/web/app/views/layout-base.pug | 5 ++++- services/web/app/views/user/settings.pug | 3 +++ 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/services/web/app/src/Features/User/UserPagesController.js b/services/web/app/src/Features/User/UserPagesController.js index c54140cded..ed2d5ce9d6 100644 --- a/services/web/app/src/Features/User/UserPagesController.js +++ b/services/web/app/src/Features/User/UserPagesController.js @@ -138,6 +138,10 @@ async function settingsPage(req, res) { ) } + // Get the user's assignment for the Bootstrap 5 split test, which populates + // splitTestVariants with a value for 'bootstrap-5' and allows Pug to read it + await SplitTestHandler.promises.getAssignment(req, res, 'bootstrap-5') + res.render('user/settings', { title: 'account_settings', user: { diff --git a/services/web/app/src/infrastructure/ExpressLocals.js b/services/web/app/src/infrastructure/ExpressLocals.js index 30527fe04c..17f375598d 100644 --- a/services/web/app/src/infrastructure/ExpressLocals.js +++ b/services/web/app/src/infrastructure/ExpressLocals.js @@ -204,10 +204,12 @@ module.exports = function (webRouter, privateApiRouter, publicApiRouter) { return staticFilesBase + webpackManifest[cssFileName] } - res.locals.buildCssPath = function (themeModifier = '') { + res.locals.buildCssPath = function ( + themeModifier = '', + bootstrapVersion = 3 + ) { // Pick which main stylesheet to use based on Bootstrap version - const bootstrap5Modifier = - res.locals.bootstrapVersion === 5 ? '-bootstrap-5' : '' + const bootstrap5Modifier = bootstrapVersion === 5 ? '-bootstrap-5' : '' return res.locals.buildStylesheetPath( `main-${themeModifier}style${bootstrap5Modifier}.css` @@ -360,13 +362,6 @@ module.exports = function (webRouter, privateApiRouter, publicApiRouter) { next() }) - webRouter.use(function (req, res, next) { - // Set the Bootstrap version to 3 in all cases for now. This will come from - // a split test/feature flag in future. - res.locals.bootstrapVersion = 3 - next() - }) - webRouter.use(function (req, res, next) { res.locals.ExposedSettings = { isOverleaf: Settings.overleaf != null, diff --git a/services/web/app/views/layout-base.pug b/services/web/app/views/layout-base.pug index e9382c5e6f..eeb79d09dc 100644 --- a/services/web/app/views/layout-base.pug +++ b/services/web/app/views/layout-base.pug @@ -4,6 +4,7 @@ html( class=(fixedSizeDocument ? 'fixed-size-document' : undefined) ) - metadata = metadata || {} + - bootstrap5EnabledPage = false block entrypointVar @@ -11,9 +12,11 @@ html( head include ./_metadata.pug + + - var bootstrapVersion = bootstrap5EnabledPage && splitTestVariants['bootstrap-5'] === 'enabled' ? 5 : 3 //- Stylesheet - link(rel='stylesheet', href=buildCssPath(getCssThemeModifier(userSettings, brandVariation)), id="main-stylesheet") + link(rel='stylesheet', href=buildCssPath(getCssThemeModifier(userSettings, brandVariation), bootstrapVersion), id="main-stylesheet") block css each file in entrypointStyles(entrypoint) link(rel='stylesheet', href=file) diff --git a/services/web/app/views/user/settings.pug b/services/web/app/views/user/settings.pug index 37803aa243..571e37e73b 100644 --- a/services/web/app/views/user/settings.pug +++ b/services/web/app/views/user/settings.pug @@ -2,6 +2,9 @@ extends ../layout-marketing block entrypointVar - entrypoint = 'pages/user/settings' + +block vars + - bootstrap5EnabledPage = true block append meta meta(name="ol-hasPassword" data-type="boolean" content=hasPassword)