From 34d267deccaad811b7a188425221b1f4533cf12a Mon Sep 17 00:00:00 2001 From: Steve Larson <9larsons@gmail.com> Date: Mon, 18 Nov 2024 11:45:03 -0600 Subject: [PATCH 1/2] Reverted custom fonts flag (#21645) ref https://ghost.slack.com/archives/C025584CA/p1731950126867179 - moved custom fonts functionality back to a labs flag - reverted gscan version to 4.45 which doesn't include the custom fonts checks/warnings --- .../settings/advanced/labs/AlphaFeatures.tsx | 4 + .../site/designAndBranding/GlobalSettings.tsx | 91 ++++++++++--------- .../site/designAndBranding/ThemeSettings.tsx | 10 +- .../test/acceptance/site/design.test.ts | 3 + .../core/core/frontend/helpers/body_class.js | 42 +++++---- .../core/core/frontend/helpers/ghost_head.js | 40 ++++---- ghost/core/core/shared/labs.js | 3 +- ghost/core/package.json | 2 +- .../admin/__snapshots__/config.test.js.snap | 1 + .../__snapshots__/ghost_head.test.js.snap | 12 +-- .../unit/frontend/helpers/body_class.test.js | 18 +++- .../unit/frontend/helpers/ghost_head.test.js | 10 ++ yarn.lock | 8 +- 13 files changed, 143 insertions(+), 101 deletions(-) diff --git a/apps/admin-x-settings/src/components/settings/advanced/labs/AlphaFeatures.tsx b/apps/admin-x-settings/src/components/settings/advanced/labs/AlphaFeatures.tsx index e640d587b5..7e38884a47 100644 --- a/apps/admin-x-settings/src/components/settings/advanced/labs/AlphaFeatures.tsx +++ b/apps/admin-x-settings/src/components/settings/advanced/labs/AlphaFeatures.tsx @@ -63,6 +63,10 @@ const features = [{ title: 'Staff 2FA', description: 'Enables email verification for staff logins', flag: 'staff2fa' +}, { + title: 'Custom Fonts', + description: 'Enables new custom font settings', + flag: 'customFonts' }]; const AlphaFeatures: React.FC = () => { diff --git a/apps/admin-x-settings/src/components/settings/site/designAndBranding/GlobalSettings.tsx b/apps/admin-x-settings/src/components/settings/site/designAndBranding/GlobalSettings.tsx index ff6da175bd..1b62a06389 100644 --- a/apps/admin-x-settings/src/components/settings/site/designAndBranding/GlobalSettings.tsx +++ b/apps/admin-x-settings/src/components/settings/site/designAndBranding/GlobalSettings.tsx @@ -1,3 +1,4 @@ +import BehindFeatureFlag from '../../../BehindFeatureFlag'; import React, {useState} from 'react'; import UnsplashSelector from '../../../selectors/UnsplashSelector'; import clsx from 'clsx'; @@ -318,50 +319,52 @@ const GlobalSettings: React.FC<{ values: GlobalSettingValues, updateSetting: (ke } -
- { - if (option?.value === DEFAULT_FONT) { - setBodyFont({name: DEFAULT_FONT, creator: themeNameVersion}); - updateSetting('body_font', ''); - } else { - setBodyFont({name: option?.value || '', creator: CUSTOM_FONTS.body.find(f => f.name === option?.value)?.creator || ''}); - updateSetting('body_font', option?.value || ''); - } - }} - /> -
+ +
+ { + if (option?.value === DEFAULT_FONT) { + setBodyFont({name: DEFAULT_FONT, creator: themeNameVersion}); + updateSetting('body_font', ''); + } else { + setBodyFont({name: option?.value || '', creator: CUSTOM_FONTS.body.find(f => f.name === option?.value)?.creator || ''}); + updateSetting('body_font', option?.value || ''); + } + }} + /> +
+ ); }; diff --git a/apps/admin-x-settings/src/components/settings/site/designAndBranding/ThemeSettings.tsx b/apps/admin-x-settings/src/components/settings/site/designAndBranding/ThemeSettings.tsx index 96181906d2..dddd00f4e8 100644 --- a/apps/admin-x-settings/src/components/settings/site/designAndBranding/ThemeSettings.tsx +++ b/apps/admin-x-settings/src/components/settings/site/designAndBranding/ThemeSettings.tsx @@ -1,5 +1,6 @@ import React from 'react'; import ThemeSetting from './ThemeSetting'; +import useFeatureFlag from '../../../../hooks/useFeatureFlag'; import {CustomThemeSetting} from '@tryghost/admin-x-framework/api/customThemeSettings'; import {Form} from '@tryghost/admin-x-design-system'; import {Theme, useBrowseThemes} from '@tryghost/admin-x-framework/api/themes'; @@ -43,6 +44,7 @@ const ThemeSettings: React.FC = ({sections, updateSetting}) const {data: themesData} = useBrowseThemes(); const activeTheme = themesData?.themes.find((theme: Theme) => theme.active); const activeThemeName = activeTheme?.package.name?.toLowerCase() || ''; + const hasCustomFonts = useFeatureFlag('customFonts'); return ( <> @@ -65,9 +67,11 @@ const ThemeSettings: React.FC = ({sections, updateSetting}) // hides typography related theme settings from official themes // should be removed once we remove the settings from the themes in 6.0 - const hidingSettings = themeSettingsMap[activeThemeName]; - if (hidingSettings && hidingSettings.includes(setting.key)) { - spaceClass += ' hidden'; + if (hasCustomFonts) { + const hidingSettings = themeSettingsMap[activeThemeName]; + if (hidingSettings && hidingSettings.includes(setting.key)) { + spaceClass += ' hidden'; + } } previousType = setting.type; diff --git a/apps/admin-x-settings/test/acceptance/site/design.test.ts b/apps/admin-x-settings/test/acceptance/site/design.test.ts index b590b2e5ce..0159600300 100644 --- a/apps/admin-x-settings/test/acceptance/site/design.test.ts +++ b/apps/admin-x-settings/test/acceptance/site/design.test.ts @@ -3,6 +3,7 @@ import { mockApi, mockSitePreview, responseFixtures, + toggleLabsFlag, updatedSettingsResponse } from '@tryghost/admin-x-framework/test/acceptance'; import {expect, test} from '@playwright/test'; @@ -305,6 +306,7 @@ test.describe('Design settings', async () => { }); test('Custom fonts', async ({page}) => { + toggleLabsFlag('customFonts', true); await mockApi({page, requests: { ...globalDataRequests, browseCustomThemeSettings: {method: 'GET', path: '/custom_theme_settings/', response: { @@ -350,6 +352,7 @@ test.describe('Design settings', async () => { }); test('Custom fonts setting back to default', async ({page}) => { + toggleLabsFlag('customFonts', true); await mockApi({page, requests: { ...globalDataRequests, browseSettings: {...globalDataRequests.browseSettings, response: updatedSettingsResponse([ diff --git a/ghost/core/core/frontend/helpers/body_class.js b/ghost/core/core/frontend/helpers/body_class.js index a734a49b62..e1d5bbca69 100644 --- a/ghost/core/core/frontend/helpers/body_class.js +++ b/ghost/core/core/frontend/helpers/body_class.js @@ -2,7 +2,7 @@ // Usage: `{{body_class}}` // // Output classes for the body element -const {settingsCache} = require('../services/proxy'); +const {labs, settingsCache} = require('../services/proxy'); const {generateCustomFontBodyClass, isValidCustomFont, isValidCustomHeadingFont} = require('@tryghost/custom-fonts'); const {SafeString} = require('../services/handlebars'); @@ -45,28 +45,30 @@ module.exports = function body_class(options) { // eslint-disable-line camelcase classes.push('paged'); } - // Check if if the request is for a site preview, in which case we **always** use the custom font values - // from the passed in data, even when they're empty strings or settings cache has values. - const isSitePreview = options.data?.site?._preview ?? false; - // Taking the fonts straight from the passed in data, as they can't be used from the - // settings cache for the theme preview until the settings are saved. Once saved, - // we need to use the settings cache to provide the correct CSS injection. - const headingFont = isSitePreview ? options.data?.site?.heading_font : settingsCache.get('heading_font'); - const bodyFont = isSitePreview ? options.data?.site?.body_font : settingsCache.get('body_font'); + if (labs.isSet('customFonts')) { + // Check if if the request is for a site preview, in which case we **always** use the custom font values + // from the passed in data, even when they're empty strings or settings cache has values. + const isSitePreview = options.data.site._preview; + // Taking the fonts straight from the passed in data, as they can't be used from the + // settings cache for the theme preview until the settings are saved. Once saved, + // we need to use the settings cache to provide the correct CSS injection. + const headingFont = isSitePreview ? options.data.site.heading_font : settingsCache.get('heading_font'); + const bodyFont = isSitePreview ? options.data.site.body_font : settingsCache.get('body_font'); - if ((typeof headingFont === 'string' && isValidCustomHeadingFont(headingFont)) || - (typeof bodyFont === 'string' && isValidCustomFont(bodyFont))) { - /** @type FontSelection */ - const fontSelection = {}; + if ((typeof headingFont === 'string' && isValidCustomHeadingFont(headingFont)) || + (typeof bodyFont === 'string' && isValidCustomFont(bodyFont))) { + /** @type FontSelection */ + const fontSelection = {}; - if (headingFont) { - fontSelection.heading = headingFont; + if (headingFont) { + fontSelection.heading = headingFont; + } + if (bodyFont) { + fontSelection.body = bodyFont; + } + const customBodyClasses = generateCustomFontBodyClass(fontSelection); + classes.push(new SafeString(customBodyClasses)); } - if (bodyFont) { - fontSelection.body = bodyFont; - } - const customBodyClasses = generateCustomFontBodyClass(fontSelection); - classes.push(new SafeString(customBodyClasses)); } classes = classes.join(' ').trim(); diff --git a/ghost/core/core/frontend/helpers/ghost_head.js b/ghost/core/core/frontend/helpers/ghost_head.js index 113df61aae..3d2a004c5a 100644 --- a/ghost/core/core/frontend/helpers/ghost_head.js +++ b/ghost/core/core/frontend/helpers/ghost_head.js @@ -353,27 +353,29 @@ module.exports = async function ghost_head(options) { // eslint-disable-line cam head.push(getTinybirdTrackerScript(dataRoot)); } - // Check if if the request is for a site preview, in which case we **always** use the custom font values - // from the passed in data, even when they're empty strings or settings cache has values. - const isSitePreview = options.data?.site?._preview ?? false; - // Taking the fonts straight from the passed in data, as they can't be used from the - // settings cache for the theme preview until the settings are saved. Once saved, - // we need to use the settings cache to provide the correct CSS injection. - const headingFont = isSitePreview ? options.data?.site?.heading_font : settingsCache.get('heading_font'); - const bodyFont = isSitePreview ? options.data?.site?.body_font : settingsCache.get('body_font'); - if ((typeof headingFont === 'string' && isValidCustomHeadingFont(headingFont)) || - (typeof bodyFont === 'string' && isValidCustomFont(bodyFont))) { - /** @type FontSelection */ - const fontSelection = {}; + if (labs.isSet('customFonts')) { + // Check if if the request is for a site preview, in which case we **always** use the custom font values + // from the passed in data, even when they're empty strings or settings cache has values. + const isSitePreview = options.data.site._preview; + // Taking the fonts straight from the passed in data, as they can't be used from the + // settings cache for the theme preview until the settings are saved. Once saved, + // we need to use the settings cache to provide the correct CSS injection. + const headingFont = isSitePreview ? options.data.site.heading_font : settingsCache.get('heading_font'); + const bodyFont = isSitePreview ? options.data.site.body_font : settingsCache.get('body_font'); + if ((typeof headingFont === 'string' && isValidCustomHeadingFont(headingFont)) || + (typeof bodyFont === 'string' && isValidCustomFont(bodyFont))) { + /** @type FontSelection */ + const fontSelection = {}; - if (headingFont) { - fontSelection.heading = headingFont; + if (headingFont) { + fontSelection.heading = headingFont; + } + if (bodyFont) { + fontSelection.body = bodyFont; + } + const customCSS = generateCustomFontCss(fontSelection); + head.push(new SafeString(customCSS)); } - if (bodyFont) { - fontSelection.body = bodyFont; - } - const customCSS = generateCustomFontCss(fontSelection); - head.push(new SafeString(customCSS)); } } diff --git a/ghost/core/core/shared/labs.js b/ghost/core/core/shared/labs.js index 4d2c76fab3..461c11e6db 100644 --- a/ghost/core/core/shared/labs.js +++ b/ghost/core/core/shared/labs.js @@ -46,7 +46,8 @@ const ALPHA_FEATURES = [ 'adminXDemo', 'contentVisibility', 'commentImprovements', - 'staff2fa' + 'staff2fa', + 'customFonts' ]; module.exports.GA_KEYS = [...GA_FEATURES]; diff --git a/ghost/core/package.json b/ghost/core/package.json index f1a80b65dc..fbc8483982 100644 --- a/ghost/core/package.json +++ b/ghost/core/package.json @@ -192,7 +192,7 @@ "ghost-storage-base": "1.0.0", "glob": "8.1.0", "got": "11.8.6", - "gscan": "4.46.0", + "gscan": "4.45.0", "human-number": "2.0.4", "image-size": "1.1.1", "intl": "1.2.5", diff --git a/ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snap b/ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snap index 3d86cbea3c..0f4e57852c 100644 --- a/ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snap +++ b/ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snap @@ -20,6 +20,7 @@ Object { "collectionsCard": true, "commentImprovements": true, "contentVisibility": true, + "customFonts": true, "editorExcerpt": true, "emailCustomization": true, "i18n": true, diff --git a/ghost/core/test/unit/frontend/helpers/__snapshots__/ghost_head.test.js.snap b/ghost/core/test/unit/frontend/helpers/__snapshots__/ghost_head.test.js.snap index c2ee2114bd..f686b3fa2f 100644 --- a/ghost/core/test/unit/frontend/helpers/__snapshots__/ghost_head.test.js.snap +++ b/ghost/core/test/unit/frontend/helpers/__snapshots__/ghost_head.test.js.snap @@ -1032,7 +1032,7 @@ Object { - + ", } @@ -1106,7 +1106,7 @@ Object { - + ", } @@ -1180,7 +1180,7 @@ Object { - + ", } @@ -1254,7 +1254,7 @@ Object { - + ", } @@ -1403,7 +1403,7 @@ Object { - + ", @@ -1553,7 +1553,7 @@ Object { - + ", diff --git a/ghost/core/test/unit/frontend/helpers/body_class.test.js b/ghost/core/test/unit/frontend/helpers/body_class.test.js index dec854c436..c39ac4ab2c 100644 --- a/ghost/core/test/unit/frontend/helpers/body_class.test.js +++ b/ghost/core/test/unit/frontend/helpers/body_class.test.js @@ -7,7 +7,7 @@ const body_class = require('../../../../core/frontend/helpers/body_class'); // Stubs const proxy = require('../../../../core/frontend/services/proxy'); -const {settingsCache} = proxy; +const {settingsCache, labs} = proxy; describe('{{body_class}} helper', function () { let options = {}; @@ -30,8 +30,7 @@ describe('{{body_class}} helper', function () { root: { context: [], settings: {active_theme: 'casper'} - }, - site: {} + } } }; }); @@ -164,6 +163,7 @@ describe('{{body_class}} helper', function () { describe('custom fonts', function () { let settingsCacheStub; + let labsStub; function callBodyClassWithContext(context, self) { options.data.root.context = context; @@ -174,6 +174,7 @@ describe('{{body_class}} helper', function () { } beforeEach(function () { + labsStub = sinon.stub(labs, 'isSet').withArgs('customFonts').returns(true); settingsCacheStub = sinon.stub(settingsCache, 'get'); options = { data: { @@ -215,6 +216,17 @@ describe('{{body_class}} helper', function () { rendered.string.should.equal('post-template tag-foo tag-bar gh-font-heading-space-grotesk gh-font-body-noto-sans'); }); + it('does not include custom font classes when custom fonts are not enabled', function () { + labsStub.withArgs('customFonts').returns(false); + + const rendered = callBodyClassWithContext( + ['post'], + {relativeUrl: '/my-awesome-post/', post: {tags: [{slug: 'foo'}, {slug: 'bar'}]}} + ); + + rendered.string.should.equal('post-template tag-foo tag-bar'); + }); + it('includes custom font classes for home page when set in options data object', function () { options.data.site.heading_font = 'Space Grotesk'; options.data.site.body_font = ''; diff --git a/ghost/core/test/unit/frontend/helpers/ghost_head.test.js b/ghost/core/test/unit/frontend/helpers/ghost_head.test.js index e6c10fef25..839e24f9bc 100644 --- a/ghost/core/test/unit/frontend/helpers/ghost_head.test.js +++ b/ghost/core/test/unit/frontend/helpers/ghost_head.test.js @@ -1128,6 +1128,8 @@ describe('{{ghost_head}} helper', function () { describe('custom fonts', function () { it('includes custom font when set in options data object and preview is set', async function () { + sinon.stub(labs, 'isSet').withArgs('customFonts').returns(true); + const renderObject = { post: posts[1] }; @@ -1152,6 +1154,7 @@ describe('{{ghost_head}} helper', function () { }); it('includes custom font when set in settings cache and no preview', async function () { + sinon.stub(labs, 'isSet').withArgs('customFonts').returns(true); settingsCache.get.withArgs('heading_font').returns('Playfair Display'); settingsCache.get.withArgs('body_font').returns('Lora'); @@ -1171,6 +1174,8 @@ describe('{{ghost_head}} helper', function () { }); it('does not include custom font when not set', async function () { + sinon.stub(labs, 'isSet').withArgs('customFonts').returns(true); + settingsCache.get.withArgs('heading_font').returns(null); settingsCache.get.withArgs('body_font').returns(''); @@ -1190,6 +1195,8 @@ describe('{{ghost_head}} helper', function () { }); it('does not include custom font when invalid', async function () { + sinon.stub(labs, 'isSet').withArgs('customFonts').returns(true); + settingsCache.get.withArgs('heading_font').returns(null); settingsCache.get.withArgs('body_font').returns('Wendy Sans'); @@ -1216,6 +1223,7 @@ describe('{{ghost_head}} helper', function () { }); it('does not inject custom fonts when preview is set and default font was selected (empty string)', async function () { + sinon.stub(labs, 'isSet').withArgs('customFonts').returns(true); // The site has fonts set up, but we override them with Theme default fonts (empty string) settingsCache.get.withArgs('heading_font').returns('Playfair Display'); settingsCache.get.withArgs('body_font').returns('Lora'); @@ -1240,6 +1248,8 @@ describe('{{ghost_head}} helper', function () { }); it('can handle preview being set and custom font keys missing', async function () { + sinon.stub(labs, 'isSet').withArgs('customFonts').returns(true); + // The site has fonts set up, but we override them with Theme default fonts (empty string) settingsCache.get.withArgs('heading_font').returns('Playfair Display'); settingsCache.get.withArgs('body_font').returns('Lora'); diff --git a/yarn.lock b/yarn.lock index a2dbc8ff89..60c65bd26d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -18792,10 +18792,10 @@ growly@^1.3.0: resolved "https://registry.yarnpkg.com/growly/-/growly-1.3.0.tgz#f10748cbe76af964b7c96c93c6bcc28af120c081" integrity sha512-+xGQY0YyAWCnqy7Cd++hc2JqMYzlm0dG30Jd0beaA64sROr8C4nt8Yc9V5Ro3avlSUDTN0ulqP/VBKi1/lLygw== -gscan@4.46.0: - version "4.46.0" - resolved "https://registry.yarnpkg.com/gscan/-/gscan-4.46.0.tgz#682e5388061e35518e0906ca1285734347cbbe60" - integrity sha512-SHsvld0EbVW7X9aHqL6P2CG31yjvrX1IxrVePZTm4yKxH7jjD1QmSmL1Ol3sFAh5MRqtGBuj9mGfDC1nJI0e7w== +gscan@4.45.0: + version "4.45.0" + resolved "https://registry.yarnpkg.com/gscan/-/gscan-4.45.0.tgz#8f033793b80ac65b64d666aac0891287c1e74909" + integrity sha512-d7yu7eGJSv7Xrd8lcBr7Bq76xeKe/LYkrRsRgE8vPRlmHxLPx7AlFb585vbS1Q64cQGwQhlAGusIeKesrfOa6w== dependencies: "@sentry/node" "^7.73.0" "@tryghost/config" "^0.2.18" From d41ccdde8dd9f1f4851f85dcc9caed306f7cf67e Mon Sep 17 00:00:00 2001 From: Ghost CI <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 18 Nov 2024 18:14:42 +0000 Subject: [PATCH 2/2] v5.101.2 --- ghost/admin/package.json | 2 +- ghost/core/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ghost/admin/package.json b/ghost/admin/package.json index 379575b3bc..eae466200a 100644 --- a/ghost/admin/package.json +++ b/ghost/admin/package.json @@ -1,6 +1,6 @@ { "name": "ghost-admin", - "version": "5.101.1", + "version": "5.101.2", "description": "Ember.js admin client for Ghost", "author": "Ghost Foundation", "homepage": "http://ghost.org", diff --git a/ghost/core/package.json b/ghost/core/package.json index fbc8483982..690d79fe14 100644 --- a/ghost/core/package.json +++ b/ghost/core/package.json @@ -1,6 +1,6 @@ { "name": "ghost", - "version": "5.101.1", + "version": "5.101.2", "description": "The professional publishing platform", "author": "Ghost Foundation", "homepage": "https://ghost.org",