From b2b398ba0dc6bc0ef0d5146f7337e5b2cd5d3da8 Mon Sep 17 00:00:00 2001 From: Tim Down <158919+timdown@users.noreply.github.com> Date: Thu, 12 Dec 2024 16:53:55 +0000 Subject: [PATCH] Merge pull request #22495 from overleaf/revert-22468-td-bs5-ieee-overall-theme Revert "Always apply overall dark theme with IEEE-branded editor and tear down ieee-stylesheet feature flag" GitOrigin-RevId: 223b4816b02ba96212ea7e779e16770cd4f16949 --- .../app/src/Features/Project/ProjectController.js | 1 + .../web/app/src/infrastructure/ExpressLocals.js | 15 ++++++++------- services/web/app/views/layout-base.pug | 8 ++++---- services/web/app/views/project/ide-react.pug | 1 - .../settings/settings-overall-theme.tsx | 7 +++++-- .../hooks/use-set-overall-theme.tsx | 4 +--- services/web/frontend/js/utils/is-ieee-branded.ts | 8 -------- 7 files changed, 19 insertions(+), 25 deletions(-) delete mode 100644 services/web/frontend/js/utils/is-ieee-branded.ts diff --git a/services/web/app/src/Features/Project/ProjectController.js b/services/web/app/src/Features/Project/ProjectController.js index 18c1619302..a503e9e490 100644 --- a/services/web/app/src/Features/Project/ProjectController.js +++ b/services/web/app/src/Features/Project/ProjectController.js @@ -347,6 +347,7 @@ const _ProjectController = { !anonymous && 'ro-mirror-on-client', 'track-pdf-download', !anonymous && 'writefull-oauth-promotion', + 'ieee-stylesheet', 'write-and-cite', 'write-and-cite-ars', 'default-visual-for-beginners', diff --git a/services/web/app/src/infrastructure/ExpressLocals.js b/services/web/app/src/infrastructure/ExpressLocals.js index d4bd6ece00..975ca7d07c 100644 --- a/services/web/app/src/infrastructure/ExpressLocals.js +++ b/services/web/app/src/infrastructure/ExpressLocals.js @@ -189,13 +189,13 @@ module.exports = function (webRouter, privateApiRouter, publicApiRouter) { res.locals.getCssThemeModifier = function ( userSettings, brandVariation, - enableIeeeBranding + ieeeStylesheetEnabled ) { // Themes only exist in OL v2 if (Settings.overleaf != null) { - // The IEEE theme is no longer applied in the editor, which sets - // enableIeeeBranding to false, but is used in the IEEE portal - if (enableIeeeBranding && res.locals.isIEEE(brandVariation)) { + // The IEEE theme takes precedence over the user personal setting, i.e. a user with + // a theme setting of "light" will still get the IEE theme in IEEE branded projects. + if (ieeeStylesheetEnabled && res.locals.isIEEE(brandVariation)) { return 'ieee-' } else if (userSettings && userSettings.overallTheme != null) { return userSettings.overallTheme @@ -213,10 +213,11 @@ module.exports = function (webRouter, privateApiRouter, publicApiRouter) { bootstrapVersion = 3 ) { // Pick which main stylesheet to use based on Bootstrap version + const bootstrap5Modifier = bootstrapVersion === 5 ? '-bootstrap-5' : '' + const computedThemeModifier = bootstrapVersion === 5 ? '' : themeModifier + return res.locals.buildStylesheetPath( - bootstrapVersion === 5 - ? 'main-style-bootstrap-5.css' - : `main-${themeModifier}style.css` + `main-${computedThemeModifier}style${bootstrap5Modifier}.css` ) } diff --git a/services/web/app/views/layout-base.pug b/services/web/app/views/layout-base.pug index 42db767bb9..8ff0f4a3b7 100644 --- a/services/web/app/views/layout-base.pug +++ b/services/web/app/views/layout-base.pug @@ -9,8 +9,7 @@ html( - let bootstrap5PageStatus = 'disabled' // One of 'disabled', 'enabled', and 'queryStringOnly' - let bootstrap5PageSplitTest = '' // Limits Bootstrap 5 usage on this page to users with an assignment of "enabled" for the specified split test. If left empty and bootstrap5PageStatus is "enabled", the page always uses Bootstrap 5. - let isWebsiteRedesign = false - - let isApplicationPage = false - - let enableIeeeBranding = true + - let isApplicationPage = false block entrypointVar @@ -22,9 +21,10 @@ html( include ./_metadata.pug - const bootstrapVersion = bootstrap5PageStatus !== 'disabled' && (bootstrap5Override || (bootstrap5PageStatus === 'enabled' && (bootstrap5PageSplitTest === '' || splitTestVariants[bootstrap5PageSplitTest] === 'enabled'))) ? 5 : 3 - + - const ieeeStylesheetEnabled = splitTestVariants?.['ieee-stylesheet'] !== 'disabled' + //- Stylesheet - link(rel='stylesheet', href=buildCssPath(getCssThemeModifier(userSettings, brandVariation, enableIeeeBranding), bootstrapVersion), id="main-stylesheet") + link(rel='stylesheet', href=buildCssPath(getCssThemeModifier(userSettings, brandVariation, ieeeStylesheetEnabled), bootstrapVersion), id="main-stylesheet") block css each file in entrypointStyles(entrypoint) link(rel='stylesheet', href=file) diff --git a/services/web/app/views/project/ide-react.pug b/services/web/app/views/project/ide-react.pug index 4bbf311d97..d0f18d2260 100644 --- a/services/web/app/views/project/ide-react.pug +++ b/services/web/app/views/project/ide-react.pug @@ -8,7 +8,6 @@ block vars - bootstrap5PageStatus = 'enabled' // One of 'disabled', 'enabled', and 'queryStringOnly' - bootstrap5PageSplitTest = 'bootstrap-5-ide' - metadata.robotsNoindexNofollow = true - - enableIeeeBranding = false block entrypointVar - entrypoint = 'pages/ide' diff --git a/services/web/frontend/js/features/editor-left-menu/components/settings/settings-overall-theme.tsx b/services/web/frontend/js/features/editor-left-menu/components/settings/settings-overall-theme.tsx index 9e2d846a35..dbf36e8e9a 100644 --- a/services/web/frontend/js/features/editor-left-menu/components/settings/settings-overall-theme.tsx +++ b/services/web/frontend/js/features/editor-left-menu/components/settings/settings-overall-theme.tsx @@ -6,7 +6,6 @@ import SettingsMenuSelect, { Option } from './settings-menu-select' import { useProjectSettingsContext } from '../../context/project-settings-context' import type { OverallThemeMeta } from '../../../../../../types/project-settings' import type { OverallTheme } from '../../../source-editor/extensions/theme' -import { isIEEEBranded } from '@/utils/is-ieee-branded' export default function SettingsOverallTheme() { const { t } = useTranslation() @@ -25,7 +24,11 @@ export default function SettingsOverallTheme() { [overallThemes] ) - if (!overallThemes || isIEEEBranded()) { + const brandVariation = getMeta('ol-brandVariation') + const { ieeeBrandId } = getMeta('ol-ExposedSettings') + const isIEEEBranded = brandVariation?.brand_id === ieeeBrandId + + if (!overallThemes || isIEEEBranded) { return null } diff --git a/services/web/frontend/js/features/editor-left-menu/hooks/use-set-overall-theme.tsx b/services/web/frontend/js/features/editor-left-menu/hooks/use-set-overall-theme.tsx index 6b696f4474..947bb9bc8d 100644 --- a/services/web/frontend/js/features/editor-left-menu/hooks/use-set-overall-theme.tsx +++ b/services/web/frontend/js/features/editor-left-menu/hooks/use-set-overall-theme.tsx @@ -7,7 +7,6 @@ import { UserSettings } from '../../../../../types/user-settings' import { useUserSettingsContext } from '@/shared/context/user-settings-context' import getMeta from '@/utils/meta' import { isBootstrap5 } from '@/features/utils/bootstrap-5' -import { isIEEEBranded } from '@/utils/is-ieee-branded' export default function useSetOverallTheme() { const [chosenTheme, setChosenTheme] = useState(null) @@ -29,8 +28,7 @@ export default function useSetOverallTheme() { useEffect(() => { // Sets `data-theme` attribute to the body element, needed for Bootstrap 5 theming - const theme = - overallTheme === 'light-' && !isIEEEBranded() ? 'light' : 'default' + const theme = overallTheme === 'light-' ? 'light' : 'default' document.body.dataset.theme = theme }, [overallTheme]) diff --git a/services/web/frontend/js/utils/is-ieee-branded.ts b/services/web/frontend/js/utils/is-ieee-branded.ts deleted file mode 100644 index 6952b4e05a..0000000000 --- a/services/web/frontend/js/utils/is-ieee-branded.ts +++ /dev/null @@ -1,8 +0,0 @@ -import getMeta from '@/utils/meta' - -export function isIEEEBranded() { - const brandVariation = getMeta('ol-brandVariation') - const { ieeeBrandId } = getMeta('ol-ExposedSettings') - - return brandVariation?.brand_id === ieeeBrandId -}