Skip to content

Commit c6d8f88

Browse files
refactor: address pr comments.
1 parent 8c7b6b2 commit c6d8f88

11 files changed

Lines changed: 172 additions & 69 deletions

File tree

playwright/rendering-modes/core.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ test('workspace font CSS URL applies to preview and persists per workspace', asy
243243
await page.getByRole('button', { name: 'Workspaces' }).click()
244244

245245
const fontCssUrlInput = page.getByRole('textbox', {
246-
name: 'Workspace Font CSS URL',
246+
name: 'Workspace Google Fonts CSS URL',
247247
})
248248
await fontCssUrlInput.fill(
249249
'https://fonts.googleapis.com/css2?family=IBM+Plex+Sans:wght@400;700&display=swap',

src/app.js

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ import { ensureJsxTransformSource } from './modules/preview/jsx-transform-runtim
7979
import { createEditorPoolManager } from './modules/editor/editor-pool-manager.js'
8080
import { createWorkspaceTabsState } from './modules/workspace/workspace-tabs-state.js'
8181
import { createWorkspacesDrawer } from './modules/workspace/workspaces-drawer/drawer.js'
82+
import { createApplyWorkspaceFontCssUrl } from './modules/app-core/workspace-font-css-url-load.js'
83+
import { createPreviewFontSetup } from './modules/app-core/preview-font-setup.js'
8284
import {
8385
createDebouncedWorkspaceSaver,
8486
createWorkspaceStorageAdapter,
@@ -356,18 +358,12 @@ const previewBackground = createPreviewBackgroundController({
356358
},
357359
})
358360

359-
const previewFont = createPreviewFontController({
361+
const previewFont = createPreviewFontSetup({
362+
createPreviewFontController,
360363
previewFontCssUrlInput: workspacesFontCssUrlInput,
361-
getDefaultPreviewFontCssUrl: () => defaultPreviewFontCssUrl,
362-
onFontConfigChange: ({ fontCssUrl, fontFamily }) => {
363-
if (renderRuntime && typeof renderRuntime.updatePreviewFont === 'function') {
364-
renderRuntime.updatePreviewFont({ fontCssUrl, fontFamily })
365-
}
366-
367-
if (typeof queueWorkspaceSave === 'function') {
368-
queueWorkspaceSave()
369-
}
370-
},
364+
defaultPreviewFontCssUrl,
365+
getRenderRuntime: () => renderRuntime,
366+
queueWorkspaceSave: () => queueWorkspaceSave?.(),
371367
})
372368

373369
const layoutTheme = createLayoutThemeController({
@@ -976,6 +972,13 @@ const {
976972
onWorkspaceRecordApplied: onWorkspaceRecordAppliedWithStatusMetadata,
977973
})
978974

975+
const applyWorkspaceFontCssUrl = createApplyWorkspaceFontCssUrl({
976+
previewFont,
977+
flushWorkspaceSave,
978+
normalizePreviewFontCssUrl,
979+
defaultPreviewFontCssUrl,
980+
})
981+
979982
const { syncActiveWorkspaceRepositoryScope, forkWorkspaceFromCurrentState } =
980983
createWorkspaceScopeForkActions({
981984
toNonEmptyWorkspaceText,
@@ -1250,14 +1253,7 @@ const githubWorkflows = createGitHubWorkflowsSetup({
12501253
listLocalContextRecords,
12511254
refreshLocalContextOptions,
12521255
applyWorkspaceRecord,
1253-
applyWorkspaceFontCssUrl: async fontCssUrl => {
1254-
previewFont.applyPreviewFontCssUrl(fontCssUrl, {
1255-
emitChange: true,
1256-
syncInputValue: true,
1257-
})
1258-
await flushWorkspaceSave({ preserveRecordId: true })
1259-
return true
1260-
},
1256+
applyWorkspaceFontCssUrl,
12611257
syncActiveWorkspaceRepositoryScope,
12621258
forkWorkspaceFromCurrentState,
12631259
flushWorkspaceSave,

src/index.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -811,13 +811,13 @@ <h2 id="workspaces-title">Workspaces</h2>
811811
class="github-pr-field github-pr-field--full"
812812
for="workspaces-font-css-url"
813813
>
814-
<span>Font CSS URL</span>
814+
<span>Google Fonts CSS URL</span>
815815
<input
816816
id="workspaces-font-css-url"
817817
type="text"
818818
autocomplete="off"
819819
spellcheck="false"
820-
aria-label="Workspace Font CSS URL"
820+
aria-label="Workspace Google Fonts CSS URL"
821821
placeholder="https://fonts.googleapis.com/css2?family=IBM+Plex+Sans:wght@400;700&display=swap"
822822
/>
823823
</label>

src/modules/app-core/github-workflows.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -551,11 +551,11 @@ const initializeGitHubWorkflows = ({
551551
}
552552

553553
try {
554-
await applyWorkspaceFontCssUrl(fontCssUrl)
554+
const loadResult = await applyWorkspaceFontCssUrl(fontCssUrl)
555555
if (typeof scheduleRender === 'function') {
556556
await Promise.resolve(scheduleRender())
557557
}
558-
return true
558+
return loadResult
559559
} catch {
560560
workspacesDrawerController?.setStatus('Could not load font CSS URL.', 'error')
561561
return false
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
const createPreviewFontSetup = ({
2+
createPreviewFontController,
3+
previewFontCssUrlInput,
4+
defaultPreviewFontCssUrl,
5+
getRenderRuntime,
6+
queueWorkspaceSave,
7+
}) => {
8+
return createPreviewFontController({
9+
previewFontCssUrlInput,
10+
getDefaultPreviewFontCssUrl: () => defaultPreviewFontCssUrl,
11+
onFontConfigChange: ({ fontCssUrl, fontFamily }) => {
12+
const renderRuntime =
13+
typeof getRenderRuntime === 'function' ? getRenderRuntime() : null
14+
if (renderRuntime && typeof renderRuntime.updatePreviewFont === 'function') {
15+
renderRuntime.updatePreviewFont({ fontCssUrl, fontFamily })
16+
}
17+
18+
if (typeof queueWorkspaceSave === 'function') {
19+
queueWorkspaceSave()
20+
}
21+
},
22+
})
23+
}
24+
25+
export { createPreviewFontSetup }
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
const createApplyWorkspaceFontCssUrl = ({
2+
previewFont,
3+
flushWorkspaceSave,
4+
normalizePreviewFontCssUrl,
5+
defaultPreviewFontCssUrl,
6+
}) => {
7+
return async fontCssUrl => {
8+
const requestedFontCssUrl = typeof fontCssUrl === 'string' ? fontCssUrl.trim() : ''
9+
const normalizedFontCssUrl = normalizePreviewFontCssUrl(requestedFontCssUrl)
10+
const defaultNormalizedFontCssUrl = normalizePreviewFontCssUrl(
11+
defaultPreviewFontCssUrl,
12+
)
13+
const rejectedInput =
14+
requestedFontCssUrl.length > 0 &&
15+
normalizedFontCssUrl === defaultNormalizedFontCssUrl &&
16+
requestedFontCssUrl !== defaultNormalizedFontCssUrl
17+
18+
previewFont.applyPreviewFontCssUrl(requestedFontCssUrl, {
19+
emitChange: true,
20+
syncInputValue: true,
21+
})
22+
23+
await flushWorkspaceSave({ preserveRecordId: true })
24+
25+
return {
26+
ok: !rejectedInput,
27+
status: rejectedInput
28+
? 'rejected'
29+
: requestedFontCssUrl !== normalizedFontCssUrl
30+
? 'normalized'
31+
: 'loaded',
32+
requestedFontCssUrl,
33+
normalizedFontCssUrl,
34+
}
35+
}
36+
}
37+
38+
export { createApplyWorkspaceFontCssUrl }

src/modules/font-css-url.js

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
const defaultFontCssUrl =
2+
'https://fonts.googleapis.com/css2?family=Inter:ital,opsz,wght@0,14..32,100..900;1,14..32,100..900&family=Roboto:ital,wght@0,100..900;1,100..900&display=swap'
3+
4+
const resolveCurrentProtocol = () => {
5+
if (typeof window === 'undefined' || !window?.location?.protocol) {
6+
return ''
7+
}
8+
9+
return String(window.location.protocol).toLowerCase()
10+
}
11+
12+
const normalizeFontCssUrl = (
13+
value,
14+
{ fallback = defaultFontCssUrl, currentProtocol = resolveCurrentProtocol() } = {},
15+
) => {
16+
const normalized = typeof value === 'string' ? value.trim() : ''
17+
if (!normalized) {
18+
return fallback
19+
}
20+
21+
try {
22+
const parsed = new URL(normalized)
23+
const protocol = parsed.protocol.toLowerCase()
24+
const allowHttp = String(currentProtocol).toLowerCase() === 'http:'
25+
if (protocol === 'http:' && !allowHttp) {
26+
return fallback
27+
}
28+
if (protocol !== 'https:' && protocol !== 'http:') {
29+
return fallback
30+
}
31+
32+
return parsed.href
33+
} catch {
34+
return fallback
35+
}
36+
}
37+
38+
export { defaultFontCssUrl, normalizeFontCssUrl }

src/modules/preview-runtime/iframe-preview-executor.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,11 @@ const createIframeShellDocument = ({ channelId, parentOrigin, importMap }) => {
100100
try {
101101
const parsed = new URL(normalized, window.location.href)
102102
const protocol = parsed.protocol.toLowerCase()
103+
const currentProtocol = String(window.location.protocol || '').toLowerCase()
104+
const allowHttp = currentProtocol === 'http:'
105+
if (protocol === 'http:' && !allowHttp) {
106+
return ''
107+
}
103108
if (protocol !== 'https:' && protocol !== 'http:') {
104109
return ''
105110
}

src/modules/preview/preview-font.js

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,12 @@
1+
import { defaultFontCssUrl, normalizeFontCssUrl } from '../font-css-url.js'
2+
13
const systemSansFontFamily =
24
'system-ui, -apple-system, "Segoe UI", Roboto, "Helvetica Neue", Arial, sans-serif'
35

4-
const defaultPreviewFontCssUrl =
5-
'https://fonts.googleapis.com/css2?family=Inter:ital,opsz,wght@0,14..32,100..900;1,14..32,100..900&family=Roboto:ital,wght@0,100..900;1,100..900&display=swap'
6-
7-
const normalizePreviewFontCssUrl = value => {
8-
const normalized = typeof value === 'string' ? value.trim() : ''
9-
if (!normalized) {
10-
return defaultPreviewFontCssUrl
11-
}
6+
const defaultPreviewFontCssUrl = defaultFontCssUrl
127

13-
try {
14-
const parsed = new URL(normalized)
15-
const protocol = parsed.protocol.toLowerCase()
16-
if (protocol !== 'https:' && protocol !== 'http:') {
17-
return defaultPreviewFontCssUrl
18-
}
19-
return parsed.href
20-
} catch {
21-
return defaultPreviewFontCssUrl
22-
}
23-
}
8+
const normalizePreviewFontCssUrl = value =>
9+
normalizeFontCssUrl(value, { fallback: defaultPreviewFontCssUrl })
2410

2511
const decodeGoogleFontsFamilyName = value => {
2612
if (typeof value !== 'string' || value.length === 0) {

src/modules/workspace/workspace-storage.js

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { cdnImports, importFromCdnWithFallback } from '../cdn.js'
2+
import { defaultFontCssUrl, normalizeFontCssUrl } from '../font-css-url.js'
23
import { toWorkspaceRecordKey } from './workspace-tab-helpers.js'
34

45
const workspaceDbName = 'knighted-develop-workspaces'
@@ -23,26 +24,8 @@ const toTabRole = value => {
2324

2425
const normalizeRenderMode = value => (value === 'react' ? 'react' : 'dom')
2526

26-
const defaultFontCssUrl =
27-
'https://fonts.googleapis.com/css2?family=Inter:ital,opsz,wght@0,14..32,100..900;1,14..32,100..900&family=Roboto:ital,wght@0,100..900;1,100..900&display=swap'
28-
29-
const normalizePreviewFontCssUrl = value => {
30-
const normalized = typeof value === 'string' ? value.trim() : ''
31-
if (!normalized) {
32-
return defaultFontCssUrl
33-
}
34-
35-
try {
36-
const parsed = new URL(normalized)
37-
const protocol = parsed.protocol.toLowerCase()
38-
if (protocol !== 'https:' && protocol !== 'http:') {
39-
return defaultFontCssUrl
40-
}
41-
return parsed.href
42-
} catch {
43-
return defaultFontCssUrl
44-
}
45-
}
27+
const normalizePreviewFontCssUrl = value =>
28+
normalizeFontCssUrl(value, { fallback: defaultFontCssUrl })
4629

4730
const toSyncTimestamp = value =>
4831
Number.isFinite(value) && value > 0 ? Math.max(0, Number(value)) : null
@@ -198,12 +181,31 @@ const backfillWorkspaceFontCssUrlField = async db => {
198181
return
199182
}
200183

201-
const recordIds = records
184+
const candidateRecordIds = records
185+
.filter(record => {
186+
if (!record || typeof record !== 'object') {
187+
return false
188+
}
189+
190+
const normalizedFontCssUrl = normalizePreviewFontCssUrl(
191+
record.fontCssUrl ?? record.previewFontCssUrl,
192+
)
193+
194+
return (
195+
!hasOwnRecordValue(record, 'fontCssUrl') ||
196+
typeof record.fontCssUrl !== 'string' ||
197+
record.fontCssUrl !== normalizedFontCssUrl
198+
)
199+
})
202200
.map(record => (typeof record?.id === 'string' ? record.id : ''))
203201
.filter(Boolean)
204202

203+
if (candidateRecordIds.length === 0) {
204+
return
205+
}
206+
205207
await Promise.all(
206-
recordIds.map(async recordId => {
208+
candidateRecordIds.map(async recordId => {
207209
const record = await db.get(workspaceStoreName, recordId)
208210
if (!record || typeof record !== 'object') {
209211
return
@@ -249,7 +251,7 @@ export const createWorkspaceStorageAdapter = ({ loadRuntime } = {}) => {
249251
})
250252
}
251253

252-
return Promise.all([dbPromise, fontCssUrlBackfillPromise]).then(([db]) => db)
254+
return dbPromise
253255
}
254256

255257
const getWorkspaceById = async id => {

0 commit comments

Comments
 (0)