feat(desktop): viewport switcher (desktop / tablet / mobile with phone bezel)#32
Merged
feat(desktop): viewport switcher (desktop / tablet / mobile with phone bezel)#32
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review mode: initial\n\nFindings\n\n- [Blocker] Hard-coded viewport geometry in the new phone frame violates the repo UI-token constraint and makes the component brittle across density/scale changes. uses literal , , , , and values instead of tokens or shared sizing primitives, despite the hard constraint that UI values come from tokens. Evidence: , \n Suggested fix:\n \n\n- [Major] Tablet preview likely collapses vertically because the wrapper only sets width, while the iframe itself uses . With no explicit height on the tablet container, the iframe’s has no definite reference and can render at an unexpected/empty height. Evidence: \n Suggested fix:\n \n\nSummary\nReview mode: initial. I found 2 issues introduced by this diff: one hard constraint violation in the new phone-frame UI, and one layout bug in tablet preview sizing.\n\nTesting\nNot run (automation). Missing coverage: no Vitest/Playwright assertions for the new viewport-specific rendering paths in .\n\nopen-codesign Bot
| display: 'inline-flex', | ||
| flexDirection: 'column', | ||
| position: 'relative', | ||
| borderRadius: 'var(--radius-phone, 44px)', |
Contributor
There was a problem hiding this comment.
Hard-coded viewport geometry in the new phone frame violates the repo UI-token constraint and makes the component brittle across density/scale changes. uses literal , , , , and values instead of tokens or shared sizing primitives.\n\nSuggested fix:\n
| ); | ||
| } else if (previewViewport === 'tablet') { | ||
| body = ( | ||
| <div className="h-full p-6 flex justify-center overflow-auto"> |
Contributor
There was a problem hiding this comment.
Tablet preview likely collapses vertically because the wrapper only sets width, while the iframe itself uses . With no explicit height on the tablet container, the iframe’s has no definite reference and can render at an unexpected/empty height.\n\nSuggested fix:\n
Contributor
There was a problem hiding this comment.
Review mode: initial
Findings
-
[Blocker] Hard-coded viewport geometry in the new phone frame violates the repo UI-token constraint and makes the component brittle across density/scale changes.
PhoneFrameuses literal8px,120px,30px,375px, and812pxvalues instead of tokens or shared sizing primitives, despite the hard constraint that UI values come frompackages/uitokens. Evidence:apps/desktop/src/renderer/src/components/PhoneFrame.tsx:19-25,apps/desktop/src/renderer/src/components/PhoneFrame.tsx:36-52
Suggested fix:const PHONE_SCREEN_WIDTH = 'var(--size-preview-mobile-width)'; const PHONE_SCREEN_HEIGHT = 'var(--size-preview-mobile-height)'; <div style={{ borderRadius: 'var(--radius-phone)', border: 'var(--border-width-strong) solid var(--color-border-strong)' }}> <div style={{ width: 'var(--size-preview-mobile-notch-width)', height: 'var(--size-preview-mobile-notch-height)' }} /> <div style={{ width: PHONE_SCREEN_WIDTH, height: PHONE_SCREEN_HEIGHT }} /> </div>
-
[Major] Tablet preview likely collapses vertically because the wrapper only sets width, while the iframe itself uses
h-full. With no explicit height on the tablet container, the iframe’sheight: 100%has no definite reference and can render at an unexpected/empty height. Evidence:apps/desktop/src/renderer/src/components/PreviewPane.tsx:120-129
Suggested fix:} else if (previewViewport === 'tablet') { body = ( <div className="h-full p-6 flex justify-center overflow-auto"> <div className="relative" style={{ width: '768px', height: '100%', flexShrink: 0 }}> <div className={COMMENT_HINT_CLASS}>{t('preview.clickToComment')}</div> {iframe} <InlineCommentComposer /> </div> </div> ); }
Summary
Review mode: initial. I found 2 issues introduced by this diff: one hard constraint violation in the new phone-frame UI, and one layout bug in tablet preview sizing.
Testing
Not run (automation). Missing coverage: no Vitest/Playwright assertions for the new viewport-specific rendering paths in PreviewPane.
open-codesign Bot
| display: 'inline-flex', | ||
| flexDirection: 'column', | ||
| position: 'relative', | ||
| borderRadius: 'var(--radius-phone, 44px)', |
Contributor
There was a problem hiding this comment.
Hard-coded viewport geometry in the new phone frame violates the repo UI-token constraint and makes the component brittle across density/scale changes. PhoneFrame uses literal 8px, 120px, 30px, 375px, and 812px values instead of tokens or shared sizing primitives.
Suggested fix:
const PHONE_SCREEN_WIDTH = 'var(--size-preview-mobile-width)';
const PHONE_SCREEN_HEIGHT = 'var(--size-preview-mobile-height)';
<div style={{ borderRadius: 'var(--radius-phone)', border: 'var(--border-width-strong) solid var(--color-border-strong)' }}>
<div style={{ width: 'var(--size-preview-mobile-notch-width)', height: 'var(--size-preview-mobile-notch-height)' }} />
<div style={{ width: PHONE_SCREEN_WIDTH, height: PHONE_SCREEN_HEIGHT }} />
</div>| ); | ||
| } else if (previewViewport === 'tablet') { | ||
| body = ( | ||
| <div className="h-full p-6 flex justify-center overflow-auto"> |
Contributor
There was a problem hiding this comment.
Tablet preview likely collapses vertically because the wrapper only sets width, while the iframe itself uses h-full. With no explicit height on the tablet container, the iframe’s height: 100% has no definite reference and can render at an unexpected/empty height.
Suggested fix:
} else if (previewViewport === 'tablet') {
body = (
<div className="h-full p-6 flex justify-center overflow-auto">
<div className="relative" style={{ width: '768px', height: '100%', flexShrink: 0 }}>
<div className={COMMENT_HINT_CLASS}>{t('preview.clickToComment')}</div>
{iframe}
<InlineCommentComposer />
</div>
</div>
);
}
Contributor
There was a problem hiding this comment.
Findings
-
[Blocker] Hard-coded mobile iframe size violates the UI-token constraint and can desync from the frame token sizes, causing clipping/misalignment when tokens change, evidence
apps/desktop/src/renderer/src/components/PreviewPane.tsx:106.
Suggested fix:const mobileSize = { width: 'var(--size-preview-mobile-width)', height: 'var(--size-preview-mobile-height)', }; <iframe ... className={isMobile ? 'w-full h-full bg-[var(--color-surface)]' : ...} style={isMobile ? mobileSize : undefined} />
-
[Major] New viewport toggle buttons introduce hard-coded pixel sizes/easing values in app code (
w-[14px],w-[30px],duration-150, literal cubic-bezier), which violates the repo rule that UI values must come frompackages/uitokens, evidenceapps/desktop/src/renderer/src/components/PreviewToolbar.tsx:79,apps/desktop/src/renderer/src/components/PreviewToolbar.tsx:117.
Suggested fix:<Monitor className="w-[var(--size-icon-sm)] h-[var(--size-icon-sm)]" aria-hidden="true" /> className={[ 'inline-flex items-center justify-center w-[var(--size-control-sm)] h-[var(--size-control-sm)]', 'transition-[background-color] duration-[var(--duration-fast)] ease-[var(--ease-out)]', ... ].join(' ')}
Summary
- Review mode: follow-up after new commits
- Found 2 issues in the current PR diff. Prior blocker on phone-frame tokenization was partially addressed, but hard-coded renderer UI values remain in added lines.
docs/VISION.mdanddocs/PRINCIPLES.md: Not found in repo/docs in this checkout.
Testing
- Not run (automation).
- Suggested coverage: add a renderer test (or Playwright assertion) that switches viewport modes and verifies token-based mobile dimensions are applied instead of literal px values.
open-codesign Bot
| ? 'w-full h-full bg-white' | ||
| : 'w-full h-full bg-white rounded-[var(--radius-2xl)] shadow-[var(--shadow-card)] border border-[var(--color-border)]' | ||
| } | ||
| style={isMobile ? { width: '375px', height: '812px' } : undefined} |
Contributor
There was a problem hiding this comment.
Hard-coded 375px/812px conflicts with the token-only UI rule and can drift from PhoneFrame if token values change.
Suggested fix:
style={
isMobile
? {
width: 'var(--size-preview-mobile-width)',
height: 'var(--size-preview-mobile-height)',
}
: undefined
}| aria-pressed={active} | ||
| onClick={() => setPreviewViewport(item.value)} | ||
| className={[ | ||
| 'inline-flex items-center justify-center w-[30px] h-[30px] transition-[background-color] duration-150 ease-[cubic-bezier(0.16,1,0.3,1)]', |
Contributor
There was a problem hiding this comment.
These new hard-coded values (w-[30px], h-[30px], duration-150, literal bezier) violate the repo UI-token constraint.
Suggested fix:
className={[
'inline-flex items-center justify-center w-[var(--size-control-sm)] h-[var(--size-control-sm)]',
'transition-[background-color] duration-[var(--duration-fast)] ease-[var(--ease-out)]',
...
].join(' ')}
Contributor
There was a problem hiding this comment.
Findings
-
[Blocker] Hard-coded color/size values were reintroduced in the preview hint badge, violating the token-only UI constraint and risking style drift across themes, evidence
apps/desktop/src/renderer/src/components/PreviewPane.tsx:35.
Suggested fix:const COMMENT_HINT_CLASS = 'absolute left-[var(--space-5)] top-[var(--space-5)] z-10 rounded-full border border-[var(--color-border)] bg-[var(--color-surface-elevated)] px-[var(--space-3)] py-[var(--space-1)] text-[var(--text-xs)] text-[var(--color-text-secondary)] shadow-[var(--shadow-soft)] backdrop-blur';
-
[Blocker] New viewport controls use hard-coded sizing and motion values (
w-[14px],w-[30px],duration-150, literal cubic-bezier), which violates the hard constraint that UI values must come frompackages/uitokens, evidenceapps/desktop/src/renderer/src/components/PreviewToolbar.tsx:79,apps/desktop/src/renderer/src/components/PreviewToolbar.tsx:117.
Suggested fix:icon: <Monitor className="w-[var(--size-icon-sm)] h-[var(--size-icon-sm)]" aria-hidden="true" />, className={[ 'inline-flex items-center justify-center w-[var(--size-control-sm)] h-[var(--size-control-sm)]', 'transition-[background-color] duration-[var(--duration-fast)] ease-[var(--ease-out)]', active ? 'bg-[var(--color-surface-active)] text-[var(--color-text-primary)]' : 'bg-[var(--color-surface)] text-[var(--color-text-muted)] hover:bg-[var(--color-surface-hover)] hover:text-[var(--color-text-secondary)]', ].join(' ')}
Summary
- Review mode: follow-up after new commits
- Found 2 blocking issues in added/modified lines of the latest PR diff.
docs/VISION.mdanddocs/PRINCIPLES.md: Not found in repo/docs.
Testing
- Not run (automation).
- Suggested tests: add a renderer component test (or Playwright flow) that toggles desktop/tablet/mobile and asserts token-driven sizing/styling classes are applied.
open-codesign Bot
| } | ||
|
|
||
| const COMMENT_HINT_CLASS = | ||
| 'absolute left-5 top-5 z-10 rounded-full border border-[var(--color-border)] bg-[rgba(255,255,255,0.88)] px-3 py-1 text-[11px] text-[var(--color-text-secondary)] shadow-[var(--shadow-soft)] backdrop-blur'; |
Contributor
There was a problem hiding this comment.
Hard-coded UI literals here (left-5, top-5, px-3, py-1, text-[11px], bg-[rgba(...)]) violate the token-only rule.
Suggested fix:
const COMMENT_HINT_CLASS =
'absolute left-[var(--space-5)] top-[var(--space-5)] ... text-[var(--text-xs)] bg-[var(--color-surface-elevated)] ...';| aria-pressed={active} | ||
| onClick={() => setPreviewViewport(item.value)} | ||
| className={[ | ||
| 'inline-flex items-center justify-center w-[30px] h-[30px] transition-[background-color] duration-150 ease-[cubic-bezier(0.16,1,0.3,1)]', |
Contributor
There was a problem hiding this comment.
This introduces hard-coded icon/control sizes and motion values (w-[14px], w-[30px], duration-150, literal bezier), which should be tokenized.
Suggested fix:
icon: <Monitor className="w-[var(--size-icon-sm)] h-[var(--size-icon-sm)]" aria-hidden="true" />,
className={[
'... w-[var(--size-control-sm)] h-[var(--size-control-sm)]',
'... duration-[var(--duration-fast)] ease-[var(--ease-out)]',
].join(' ')}
hqhq1025
added a commit
that referenced
this pull request
Apr 18, 2026
Address Codex blocker re-flagged on PR #32: - Replace hardcoded values in PreviewPane hint badge (left-5/top-5, bg-[rgba(255,255,255,0.88)], px-3/py-1, text-[11px]) with --space-5/--space-3/--space-1, --color-surface-elevated, --text-xs. - Replace hardcoded sizes/motion in PreviewToolbar viewport controls and Download button (w-[14px], w-[30px], h-[30px], duration-150, literal cubic-bezier) with --size-icon-sm, --size-control-xs, --duration-fast, --ease-out. - Add tokens: --color-surface-elevated (light + dark), --size-control-xs (30px), --size-icon-sm (14px).
Contributor
There was a problem hiding this comment.
Findings
-
[Blocker] Hard-coded border width in new
PhoneFrameviolates the token-only UI constraint and reintroduces non-tokenized sizing into renderer UI, evidenceapps/desktop/src/renderer/src/components/PhoneFrame.tsx:25.
Suggested fix:style={{ boxShadow: 'var(--shadow-elevated), var(--shadow-inset-soft)', // remove hard-coded outline: '1px solid ...' }}
-
[Minor] New viewport feature tests only state transitions in Zustand and do not verify rendering branches (
desktop/tablet/mobile) inPreviewPane, so regressions like broken frame sizing/layout can ship undetected, evidenceapps/desktop/src/renderer/src/store.test.ts:233,apps/desktop/src/renderer/src/components/PreviewPane.tsx:116.
Suggested fix:it('renders phone frame in mobile viewport', () => { useCodesignStore.setState({ previewHtml: '<html></html>', previewViewport: 'mobile' }); render(<PreviewPane onPickStarter={() => {}} />); expect(screen.getByTitle('design-preview')).toHaveClass('w-full', 'h-full'); expect(document.querySelector('[aria-hidden="true"]')).toBeTruthy(); });
Summary
- Review mode: follow-up after new commits
- Found 1 blocker and 1 minor issue in added/modified lines of the latest PR diff.
docs/VISION.mdanddocs/PRINCIPLES.md: Not found in repo/docs.
Testing
- Not run (automation)
- Suggested tests: add renderer tests for
PreviewPaneviewport branches and one Playwright flow that toggles desktop/tablet/mobile and validates visible container geometry.
open-codesign Bot
| background: 'var(--color-surface)', | ||
| overflow: 'hidden', | ||
| /* extra inner shadow mimicking the screen recess */ | ||
| outline: '1px solid var(--color-border)', |
Contributor
There was a problem hiding this comment.
[Blocker] This introduces a hard-coded 1px UI value in component code, which violates the token-only UI constraint.
Suggested fix:
style={{
boxShadow: 'var(--shadow-elevated), var(--shadow-inset-soft)',
// remove hard-coded outline
}}| }); | ||
| }); | ||
|
|
||
| describe('useCodesignStore previewViewport', () => { |
Contributor
There was a problem hiding this comment.
[Minor] Added tests only cover state transitions; they do not verify that new viewport rendering branches in PreviewPane actually render correctly.
Suggested fix:
it('renders phone frame in mobile viewport', () => {
useCodesignStore.setState({ previewHtml: '<html></html>', previewViewport: 'mobile' });
render(<PreviewPane onPickStarter={() => {}} />);
expect(screen.getByTitle('design-preview')).toHaveClass('w-full', 'h-full');
});
hqhq1025
added a commit
that referenced
this pull request
Apr 18, 2026
Address Codex blocker re-flagged on PR #32: - Replace hardcoded values in PreviewPane hint badge (left-5/top-5, bg-[rgba(255,255,255,0.88)], px-3/py-1, text-[11px]) with --space-5/--space-3/--space-1, --color-surface-elevated, --text-xs. - Replace hardcoded sizes/motion in PreviewToolbar viewport controls and Download button (w-[14px], w-[30px], h-[30px], duration-150, literal cubic-bezier) with --size-icon-sm, --size-control-xs, --duration-fast, --ease-out. - Add tokens: --color-surface-elevated (light + dark), --size-control-xs (30px), --size-icon-sm (14px). Signed-off-by: hqhq1025 <[email protected]>
hqhq1025
force-pushed
the
wt/feat-mobile-frame
branch
from
2b36497 to
1891c1a
Compare
Contributor
There was a problem hiding this comment.
Findings
-
[Major] Mobile notch blocks interactions in the top preview area — the notch overlay is rendered above the iframe with default pointer handling, so clicks/taps in that region never reach the canvas, which breaks inline selection/editing near the top. Evidence
apps/desktop/src/renderer/src/components/PhoneFrame.tsx:27
Suggested fix:<div aria-hidden="true" style={{ position: 'absolute', top: 0, left: '50%', transform: 'translateX(-50%)', width: 'var(--size-preview-mobile-notch-width)', height: 'var(--size-preview-mobile-notch-height)', background: 'var(--color-border-strong)', borderBottomLeftRadius: 'var(--radius-lg)', borderBottomRightRadius: 'var(--radius-lg)', zIndex: 2, pointerEvents: 'none', }} />
-
[Minor] Viewport icon buttons have no explicit accessible name — icon-only buttons rely on
title, which is not a reliable accessible name across assistive tech and touch contexts. Evidenceapps/desktop/src/renderer/src/components/PreviewToolbar.tsx:122
Suggested fix:<button key={item.value} type="button" title={item.label} aria-label={item.label} aria-pressed={active} onClick={() => setPreviewViewport(item.value)} className={...} > {item.icon} </button>
Summary
- Review mode: follow-up after new commits
- 2 issues found in the latest head diff (interaction regression + accessibility gap).
docs/VISION.mdanddocs/PRINCIPLES.mdare not present in this checkout (Not found in repo/docs), so architecture/principle conformance beyond the changed code could not be re-validated from those docs.
Testing
- Not run (automation)
- Suggested coverage additions: renderer component tests for viewport toggle accessibility labels and mobile-notch click-through behavior in preview.
open-codesign Bot
| }} | ||
| > | ||
| {/* Notch */} | ||
| <div |
Contributor
There was a problem hiding this comment.
This notch overlay sits above the iframe and currently captures pointer events. That blocks click/selection interactions in the top mobile preview area. Please add pointerEvents: 'none' to keep the visual bezel while preserving canvas interaction.
| <button | ||
| key={item.value} | ||
| type="button" | ||
| title={item.label} |
Contributor
There was a problem hiding this comment.
These viewport controls are icon-only buttons; title is not a reliable accessible name for screen readers/touch UIs. Please add aria-label={item.label} on each button.
hqhq1025
added a commit
that referenced
this pull request
Apr 18, 2026
Address Codex blocker re-flagged on PR #32: - Replace hardcoded values in PreviewPane hint badge (left-5/top-5, bg-[rgba(255,255,255,0.88)], px-3/py-1, text-[11px]) with --space-5/--space-3/--space-1, --color-surface-elevated, --text-xs. - Replace hardcoded sizes/motion in PreviewToolbar viewport controls and Download button (w-[14px], w-[30px], h-[30px], duration-150, literal cubic-bezier) with --size-icon-sm, --size-control-xs, --duration-fast, --ease-out. - Add tokens: --color-surface-elevated (light + dark), --size-control-xs (30px), --size-icon-sm (14px). Signed-off-by: hqhq1025 <[email protected]>
hqhq1025
force-pushed
the
wt/feat-mobile-frame
branch
from
1891c1a to
64c5df5
Compare
Contributor
There was a problem hiding this comment.
Findings
- [Minor] Viewport icon buttons are missing an explicit accessible name, so assistive tech may announce them generically (or inconsistently) despite
title. Evidenceapps/desktop/src/renderer/src/components/PreviewToolbar.tsx:122
Suggested fix:<button key={item.value} type="button" title={item.label} aria-label={item.label} aria-pressed={active} onClick={() => setPreviewViewport(item.value)} className={...} > {item.icon} </button>
Summary
- Review mode: follow-up after new commits
- 1 issue found in the latest head diff.
- Residual risk: no component-level test currently verifies accessible naming for the viewport switcher buttons.
Testing
- Not run (automation)
- Suggested tests: renderer component test that asserts each viewport control has an accessible name from i18n and remains keyboard-operable.
open-codesign Bot
| <button | ||
| key={item.value} | ||
| type="button" | ||
| title={item.label} |
Contributor
There was a problem hiding this comment.
Icon-only controls should have an explicit accessible name. title is not a reliable substitute across screen readers/touch contexts.
Suggested minimal change:
<button
...
title={item.label}
aria-label={item.label}
aria-pressed={active}
>
{item.icon}
</button>
hqhq1025
added a commit
that referenced
this pull request
Apr 18, 2026
Address Codex blocker re-flagged on PR #32: - Replace hardcoded values in PreviewPane hint badge (left-5/top-5, bg-[rgba(255,255,255,0.88)], px-3/py-1, text-[11px]) with --space-5/--space-3/--space-1, --color-surface-elevated, --text-xs. - Replace hardcoded sizes/motion in PreviewToolbar viewport controls and Download button (w-[14px], w-[30px], h-[30px], duration-150, literal cubic-bezier) with --size-icon-sm, --size-control-xs, --duration-fast, --ease-out. - Add tokens: --color-surface-elevated (light + dark), --size-control-xs (30px), --size-icon-sm (14px). Signed-off-by: hqhq1025 <[email protected]>
hqhq1025
force-pushed
the
wt/feat-mobile-frame
branch
from
64c5df5 to
e155962
Compare
Contributor
There was a problem hiding this comment.
Findings
-
[Major] Viewport icon buttons still have no explicit accessible name, so assistive tech can announce unlabeled controls despite
title; this is the same unresolved issue from the prior bot review. Evidenceapps/desktop/src/renderer/src/components/PreviewToolbar.tsx:123
Suggested fix:<button key={item.value} type="button" title={item.label} aria-label={item.label} aria-pressed={active} onClick={() => setPreviewViewport(item.value)} className={...} > {item.icon} </button>
-
[Major] Hardcoded
bg-whitewas added in the viewport refactor, which violates the project token-only UI value constraint for app code. Evidenceapps/desktop/src/renderer/src/components/PreviewPane.tsx:109andapps/desktop/src/renderer/src/components/PreviewPane.tsx:110
Suggested fix:className={ isMobile ? 'w-full h-full bg-[var(--color-surface)]' : 'w-full h-full bg-[var(--color-surface)] rounded-[var(--radius-2xl)] shadow-[var(--shadow-card)] border border-[var(--color-border)]' }
-
[Minor]
--size-icon-smand--size-icon-mdare now declared twice intokens.css, which creates drift risk and makes token source-of-truth ambiguous. Evidencepackages/ui/src/tokens.css:101(new duplicate block)
Suggested fix:/* Keep only one icon-size block (single source of truth) */ --size-icon-xs: 12px; --size-icon-sm: 14px; --size-icon-md: 16px;
Summary
- Review mode: follow-up after new commits
- 3 issues found in the latest head diff.
docs/VISION.mdanddocs/PRINCIPLES.md: Not found in repo/docs.- Residual risk: no component-level tests verify viewport-switcher accessibility and viewport-specific rendering behavior.
Testing
- Not run (automation)
- Suggested tests: Vitest/RTL test for accessible names on viewport buttons and a render test that verifies desktop/tablet/mobile classes/styles.
open-codesign Bot
| <button | ||
| key={item.value} | ||
| type="button" | ||
| title={item.label} |
Contributor
There was a problem hiding this comment.
Icon-only viewport controls need aria-label for reliable screen-reader naming; title alone is not sufficient.
Suggested minimal fix:
<button ... title={item.label} aria-label={item.label} aria-pressed={active}>
{item.icon}
</button>| srcDoc={buildSrcdoc(previewHtml)} | ||
| className={ | ||
| isMobile | ||
| ? 'w-full h-full bg-white' |
Contributor
There was a problem hiding this comment.
This introduces a hardcoded bg-white class in app UI code. The repo constraint requires token-based UI values.
Suggested fix:
className={isMobile ? 'w-full h-full bg-[var(--color-surface)]' : 'w-full h-full bg-[var(--color-surface)] ...'}| --size-accent-stripe: 3px; | ||
|
|
||
| /* Icon sizes */ | ||
| --size-icon-sm: 14px; |
Contributor
There was a problem hiding this comment.
--size-icon-sm / --size-icon-md are now duplicated in this file (also declared later), which makes token ownership ambiguous. Please keep a single icon-size block.
This was referenced Apr 18, 2026
hqhq1025
added a commit
that referenced
this pull request
Apr 18, 2026
Address Codex blocker re-flagged on PR #32: - Replace hardcoded values in PreviewPane hint badge (left-5/top-5, bg-[rgba(255,255,255,0.88)], px-3/py-1, text-[11px]) with --space-5/--space-3/--space-1, --color-surface-elevated, --text-xs. - Replace hardcoded sizes/motion in PreviewToolbar viewport controls and Download button (w-[14px], w-[30px], h-[30px], duration-150, literal cubic-bezier) with --size-icon-sm, --size-control-xs, --duration-fast, --ease-out. - Add tokens: --color-surface-elevated (light + dark), --size-control-xs (30px), --size-icon-sm (14px). Signed-off-by: hqhq1025 <[email protected]>
hqhq1025
force-pushed
the
wt/feat-mobile-frame
branch
from
e155962 to
1f21872
Compare
Contributor
There was a problem hiding this comment.
Findings
-
[Major] Viewport icon buttons are still unlabeled for assistive tech —
titleis not a reliable accessible name, so screen readers may announce generic buttons. Evidenceapps/desktop/src/renderer/src/components/PreviewToolbar.tsx:123
Suggested fix:<button key={item.value} type="button" title={item.label} aria-label={item.label} aria-pressed={active} onClick={() => setPreviewViewport(item.value)} className={...} > {item.icon} </button>
-
[Major] Hardcoded
bg-whitein preview iframe class violates the token-only UI value rule and can regress theme consistency. Evidenceapps/desktop/src/renderer/src/components/PreviewPane.tsx:109
Suggested fix:className={ isMobile ? 'w-full h-full bg-[var(--color-surface)]' : 'w-full h-full bg-[var(--color-surface)] rounded-[var(--radius-2xl)] shadow-[var(--shadow-card)] border border-[var(--color-border)]' }
-
[Minor]
--size-icon-smand--size-icon-mdwere added again, duplicating existing token declarations and creating ambiguous source-of-truth. Evidencepackages/ui/src/tokens.css:102
Suggested fix:/* Remove duplicate declarations and keep a single icon-size block */ --size-icon-xs: 12px; --size-icon-sm: 14px; --size-icon-md: 16px;
Summary
- Review mode: follow-up after new commits
- 3 issues found in the latest head diff.
docs/VISION.mdanddocs/PRINCIPLES.md: Not found in repo/docs.- Residual risk: viewport rendering/accessibility behavior is not covered by component-level tests.
Testing
- Not run (automation)
- Suggested tests: Vitest + RTL test for viewport switcher accessible names, and render tests asserting desktop/tablet/mobile container classes/styles.
open-codesign Bot
| <button | ||
| key={item.value} | ||
| type="button" | ||
| title={item.label} |
Contributor
There was a problem hiding this comment.
title does not provide a reliable accessible name for icon-only buttons. Please add aria-label={item.label} on this control.
Suggested fix:
<button
key={item.value}
type="button"
title={item.label}
aria-label={item.label}
aria-pressed={active}
onClick={() => setPreviewViewport(item.value)}
className={...}
>
{item.icon}
</button>| srcDoc={buildSrcdoc(previewHtml)} | ||
| className={ | ||
| isMobile | ||
| ? 'w-full h-full bg-white' |
Contributor
There was a problem hiding this comment.
Hardcoded bg-white violates the token-only UI values rule and can break theming consistency. Use surface token values instead.
Suggested fix:
className={
isMobile
? 'w-full h-full bg-[var(--color-surface)]'
: 'w-full h-full bg-[var(--color-surface)] rounded-[var(--radius-2xl)] shadow-[var(--shadow-card)] border border-[var(--color-border)]'
}| --size-accent-stripe: 3px; | ||
|
|
||
| /* Icon sizes */ | ||
| --size-icon-sm: 14px; |
Contributor
There was a problem hiding this comment.
This introduces duplicate --size-icon-sm / --size-icon-md token declarations (already defined later), which creates token source-of-truth ambiguity. Keep one icon-size block only.
Suggested fix:
--size-icon-xs: 12px;
--size-icon-sm: 14px;
--size-icon-md: 16px;
hqhq1025
added a commit
that referenced
this pull request
Apr 19, 2026
Address Codex blocker re-flagged on PR #32: - Replace hardcoded values in PreviewPane hint badge (left-5/top-5, bg-[rgba(255,255,255,0.88)], px-3/py-1, text-[11px]) with --space-5/--space-3/--space-1, --color-surface-elevated, --text-xs. - Replace hardcoded sizes/motion in PreviewToolbar viewport controls and Download button (w-[14px], w-[30px], h-[30px], duration-150, literal cubic-bezier) with --size-icon-sm, --size-control-xs, --duration-fast, --ease-out. - Add tokens: --color-surface-elevated (light + dark), --size-control-xs (30px), --size-icon-sm (14px). Signed-off-by: hqhq1025 <[email protected]>
hqhq1025
force-pushed
the
wt/feat-mobile-frame
branch
from
1f21872 to
55cbae9
Compare
Contributor
There was a problem hiding this comment.
Findings
-
[Major] Hardcoded iframe background color violates token-only UI values and can cause theme inconsistency, evidence
apps/desktop/src/renderer/src/components/PreviewPane.tsx:109
Suggested fix:className={ isMobile ? 'w-full h-full bg-[var(--color-surface)]' : 'w-full h-full bg-[var(--color-surface)] rounded-[var(--radius-2xl)] shadow-[var(--shadow-card)] border border-[var(--color-border)]' }
-
[Minor] Duplicate icon token declarations create ambiguous source-of-truth for design tokens, evidence
packages/ui/src/tokens.css:102
Suggested fix:/* Remove duplicate declarations in this earlier block; keep a single icon-size block */ --size-icon-xs: 12px; --size-icon-sm: 14px; --size-icon-md: 16px;
Summary
- Review mode: follow-up after new commits
- 2 issues found on the latest head diff.
docs/VISION.md: Not found in repo/docs.docs/PRINCIPLES.md: Not found in repo/docs.- Residual risk: no component-level tests for viewport UI rendering/a11y states were added in this diff.
Testing
- Not run (automation)
open-codesign Bot
| srcDoc={buildSrcdoc(previewHtml)} | ||
| className={ | ||
| isMobile | ||
| ? 'w-full h-full bg-white' |
Contributor
There was a problem hiding this comment.
[Major] bg-white is a hardcoded UI value and violates the token-only rule. Use bg-[var(--color-surface)] for both branches.
| --size-accent-stripe: 3px; | ||
|
|
||
| /* Icon sizes */ | ||
| --size-icon-sm: 14px; |
Contributor
There was a problem hiding this comment.
[Minor] --size-icon-sm/--size-icon-md are redeclared here and again later in the same file. Keep a single declaration block to avoid token drift.
- Add PreviewViewport type and previewViewport/setPreviewViewport to store - Add PhoneFrame component (CSS-only iPhone bezel, fully tokenised) - Add viewport switcher (Desktop/Tablet/Mobile) to PreviewToolbar - PreviewPane renders iframe inside PhoneFrame at 375x812 for mobile, centred 768px container for tablet, and full-width for desktop - Add preview.viewport i18n keys (en + zh-CN) - Add 4 unit tests for setPreviewViewport in store.test.ts Signed-off-by: hqhq1025 <[email protected]>
Replace hard-coded px values in PhoneFrame with CSS custom properties from packages/ui tokens.css. Add --size-preview-mobile-*, --size-preview-tablet-width, --radius-phone, and --border-width-strong tokens. Fix tablet branch in PreviewPane to propagate h-full so the iframe is no longer zero-height. Signed-off-by: hqhq1025 <[email protected]>
Signed-off-by: hqhq1025 <[email protected]>
Address Codex blocker re-flagged on PR #32: - Replace hardcoded values in PreviewPane hint badge (left-5/top-5, bg-[rgba(255,255,255,0.88)], px-3/py-1, text-[11px]) with --space-5/--space-3/--space-1, --color-surface-elevated, --text-xs. - Replace hardcoded sizes/motion in PreviewToolbar viewport controls and Download button (w-[14px], w-[30px], h-[30px], duration-150, literal cubic-bezier) with --size-icon-sm, --size-control-xs, --duration-fast, --ease-out. - Add tokens: --color-surface-elevated (light + dark), --size-control-xs (30px), --size-icon-sm (14px). Signed-off-by: hqhq1025 <[email protected]>
Replace hard-coded `outline: '1px solid ...'` with the existing `--shadow-inset-soft` token composed into boxShadow. Removes the last non-tokenized value in PhoneFrame so the component is fully driven by packages/ui tokens. Signed-off-by: hqhq1025 <[email protected]>
… none) Signed-off-by: hqhq1025 <[email protected]>
title alone is not a reliable accessible name for screen readers; add aria-label using the same i18n string so assistive tech announces Mobile/Tablet/Desktop instead of a generic button. Signed-off-by: hqhq1025 <[email protected]>
hqhq1025
force-pushed
the
wt/feat-mobile-frame
branch
from
55cbae9 to
c32c2d2
Compare
Contributor
There was a problem hiding this comment.
Findings
-
[Major] Hardcoded iframe background color (
bg-white) violates the token-only UI constraint and can drift from themed surfaces, evidenceapps/desktop/src/renderer/src/components/PreviewPane.tsx:109
Suggested fix:className={ isMobile ? 'w-full h-full bg-[var(--color-surface)]' : 'w-full h-full bg-[var(--color-surface)] rounded-[var(--radius-2xl)] shadow-[var(--shadow-card)] border border-[var(--color-border)]' }
-
[Minor]
--size-icon-sm/--size-icon-mdare redeclared, creating duplicate token sources and potential drift, evidencepackages/ui/src/tokens.css:102
Suggested fix:/* Remove the duplicate icon block added near line 101 and keep a single definition block */ --size-icon-xs: 12px; --size-icon-sm: 14px; --size-icon-md: 16px;
Summary
- Review mode: follow-up after new commits
- 2 issues found on the latest PR head diff.
docs/VISION.md: Not found in repo/docs.docs/PRINCIPLES.md: Not found in repo/docs.- Residual risk/testing gap: no component-level coverage for viewport UI rendering/a11y states in this diff.
Testing
- Not run (automation)
open-codesign Bot
| srcDoc={buildSrcdoc(previewHtml)} | ||
| className={ | ||
| isMobile | ||
| ? 'w-full h-full bg-white' |
Contributor
There was a problem hiding this comment.
[Major] bg-white is a hardcoded UI value and violates the token-only constraint. Use bg-[var(--color-surface)] in both branches.
| --size-accent-stripe: 3px; | ||
|
|
||
| /* Icon sizes */ | ||
| --size-icon-sm: 14px; |
Contributor
There was a problem hiding this comment.
[Minor] --size-icon-sm/--size-icon-md are redeclared here and again later in this file. Keep a single declaration block to avoid token drift.
Contributor
There was a problem hiding this comment.
Findings
-
[Blocker] Hardcoded UI color bypasses token system —
bg-whiteis introduced in preview iframe classes, violating the project constraint that UI values must come frompackages/uitokens, evidenceapps/desktop/src/renderer/src/components/PreviewPane.tsx:110.
Suggested fix:className={ isMobile ? 'w-full h-full bg-[var(--color-artifact-bg)]' : 'w-full h-full bg-[var(--color-artifact-bg)] rounded-[var(--radius-2xl)] shadow-[var(--shadow-card)] border border-[var(--color-border)]' }
-
[Minor] Duplicate token declarations introduced in
:root—--size-icon-sm/--size-icon-mdare added earlier and already declared later in the same block, which creates shadowed definitions and future drift risk, evidencepackages/ui/src/tokens.css:104.
Suggested fix:/* Remove the duplicated early block and keep a single canonical declaration site */ /* --size-icon-sm: 14px; */ /* --size-icon-md: 16px; */
Summary
- Review mode: follow-up after new commits
- 2 issues found in the latest diff (1 blocker, 1 minor).
- Requested context docs
docs/VISION.mdanddocs/PRINCIPLES.mdare not present in this repository path (Not found in repo/docs), so policy alignment was checked against availableCLAUDE.mdplus code-level constraints.
Testing
- Not run (automation)
open-codesign Bot
| className={ | ||
| isMobile | ||
| ? 'w-full h-full bg-[var(--color-artifact-bg)]' | ||
| : 'w-full h-full bg-white rounded-[var(--radius-2xl)] shadow-[var(--shadow-card)] border border-[var(--color-border)]' |
Contributor
There was a problem hiding this comment.
[Blocker] Hardcoded bg-white bypasses tokenized UI values. Please switch this to bg-[var(--color-artifact-bg)] (or another var(--*) token) to satisfy the UI token constraint.
Suggested fix:
className={
isMobile
? "w-full h-full bg-[var(--color-artifact-bg)]"
: "w-full h-full bg-[var(--color-artifact-bg)] rounded-[var(--radius-2xl)] shadow-[var(--shadow-card)] border border-[var(--color-border)]"
}| --size-control-lg: 48px; | ||
| --size-accent-stripe: 3px; | ||
|
|
||
| /* Icon sizes */ |
Contributor
There was a problem hiding this comment.
[Minor] --size-icon-sm/--size-icon-md are redeclared later in the same :root block, so these new declarations are duplicates. Keeping one canonical declaration avoids drift.
Suggested fix:
/* remove duplicated early declarations and keep one icon-size block */…kens (PR #32 codex follow-up)
Contributor
There was a problem hiding this comment.
Findings
- No new high-confidence issues found in the added/modified lines of the latest diff.
Summary
- Review mode: follow-up after new commits
- No actionable issues found in this revision.
docs/VISION.mdanddocs/PRINCIPLES.md: Not found in repo/docs.- Residual risk/testing gap: store-level viewport transitions are covered, but end-to-end viewport rendering behavior (desktop/tablet/mobile frame layout) is not covered by Playwright in this PR.
Testing
- Not run (automation)
open-codesign Bot
hqhq1025
added a commit
that referenced
this pull request
Apr 19, 2026
…e bezel) (#32) * feat(desktop): mobile/tablet/desktop viewport preview with phone bezel - Add PreviewViewport type and previewViewport/setPreviewViewport to store - Add PhoneFrame component (CSS-only iPhone bezel, fully tokenised) - Add viewport switcher (Desktop/Tablet/Mobile) to PreviewToolbar - PreviewPane renders iframe inside PhoneFrame at 375x812 for mobile, centred 768px container for tablet, and full-width for desktop - Add preview.viewport i18n keys (en + zh-CN) - Add 4 unit tests for setPreviewViewport in store.test.ts Signed-off-by: hqhq1025 <[email protected]> * fix(ui): tokenize phone frame dimensions + give tablet wrapper a height Replace hard-coded px values in PhoneFrame with CSS custom properties from packages/ui tokens.css. Add --size-preview-mobile-*, --size-preview-tablet-width, --radius-phone, and --border-width-strong tokens. Fix tablet branch in PreviewPane to propagate h-full so the iframe is no longer zero-height. Signed-off-by: hqhq1025 <[email protected]> * fix(ui): tokenize preview viewport sizes (mobile/tablet/desktop) Signed-off-by: hqhq1025 <[email protected]> * fix(ui): tokenize preview hint badge color/size Address Codex blocker re-flagged on PR #32: - Replace hardcoded values in PreviewPane hint badge (left-5/top-5, bg-[rgba(255,255,255,0.88)], px-3/py-1, text-[11px]) with --space-5/--space-3/--space-1, --color-surface-elevated, --text-xs. - Replace hardcoded sizes/motion in PreviewToolbar viewport controls and Download button (w-[14px], w-[30px], h-[30px], duration-150, literal cubic-bezier) with --size-icon-sm, --size-control-xs, --duration-fast, --ease-out. - Add tokens: --color-surface-elevated (light + dark), --size-control-xs (30px), --size-icon-sm (14px). Signed-off-by: hqhq1025 <[email protected]> * fix(ui): tokenize PhoneFrame border + remaining hardcoded values Replace hard-coded `outline: '1px solid ...'` with the existing `--shadow-inset-soft` token composed into boxShadow. Removes the last non-tokenized value in PhoneFrame so the component is fully driven by packages/ui tokens. Signed-off-by: hqhq1025 <[email protected]> * fix(ui): make PhoneFrame notch overlay click-through (pointer-events: none) Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): add aria-label to viewport switcher buttons title alone is not a reliable accessible name for screen readers; add aria-label using the same i18n string so assistive tech announces Mobile/Tablet/Desktop instead of a generic button. Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): tokenize iframe background in mobile preview (PR #32 codex Major) * fix(desktop): tokenize desktop preview iframe bg + dedup icon size tokens (PR #32 codex follow-up) --------- Signed-off-by: hqhq1025 <[email protected]>
hqhq1025
added a commit
that referenced
this pull request
Apr 19, 2026
* feat(core): system prompt — adopt Claude Design patterns Adds a new prompt section that embeds high-leverage craft directives paraphrased from patterns observed in the publicly leaked Claude Design system prompt. Direct response to feedback that generated designs are sparse, generic, and monotone. The new section sits between tweaks-protocol and anti-slop in the composer, applies to all create/revise/tweak modes, and codifies: - silent artifact-type classification (landing / dashboard / case study / pricing / deck / etc.) controlling section ladder and density target - density floor — default to "rich", drop only on explicit "minimal" - real, specific content — concrete bans on common placeholder strings - before/after side-by-side rendering when comparison is implied - big-number visual blocks (display weight + label + delta + sparkline) - three-family typography ladder (display + sans + mono) - dark-theme warmth requirements (accent + gradient + transparent borders) - SVG monogram/wordmark for logos (no emoji, no flat circles) - distinguished customer-quote treatment - single-page structure ladder (hero → trust → 3-5 sections → focal → closing CTA), with dashboard and slide deck substitutions Directives are independently authored — no original Claude Design text is reproduced verbatim. Attribution and the underlying structural analysis live in docs/research/15-claude-design-prompts.md (gitignored, internal). Vitest coverage: - new test asserts all ten directive headers appear in the create-mode composed prompt - existing drift test ensures the new .txt and the inlined TS constant stay byte-identical Signed-off-by: hqhq1025 <[email protected]> * fix(core): rename 'Claude-Design-style' → 'Craft directives' to remove provenance comment per codex review Signed-off-by: hqhq1025 <[email protected]> * test(core): cover revise mode for craft directives (PR #43 codex Minor) * fix(desktop): replace hardcoded text-[Npx] with --text-* tokens in preview/inline-comment (#57) Codex has flagged this exact pattern as a Blocker on PRs #50 and #51 ("hardcoded pixel font size violates token-only UI constraint"). Sweeps the chat-adjacent surface that is not covered by any in-flight worktree. Mappings: - text-[11px] -> text-[var(--text-xs)] (12px, closest token) - text-[12px] -> text-[var(--text-xs)] - text-[13px] -> text-[var(--text-sm)] Files touched: InlineCommentComposer.tsx, PreviewToolbar.tsx, PreviewPane.tsx. The bg-[rgba(255,255,255,0.88)] frosted pill in PreviewPane is left for a follow-up because it requires a new translucent surface token. * fix(desktop): localize Toast dismiss button aria-label (#56) Codex flagged a recurring i18n violation pattern across PRs #48 and #49: hardcoded English ARIA strings bypass the i18n layer and ship inconsistent screen-reader output to localized users. The toast close button was the last remaining hardcoded aria-label outside the onboarding flow. - Add common.dismissNotification key (en + zh-CN) - Wire useT() into ToastItem and consume the new key * fix(desktop): tokenize ConnectionStatusDot tooltip values (#58) * chore(desktop): biome auto-format InlineCommentComposer (unblock pre-push) * fix(desktop): tokenize ConnectionStatusDot tooltip hardcoded values * fix(desktop): tokenize LanguageToggle hardcoded sizes/spacing (#60) * fix(desktop): tokenize LanguageToggle hardcoded sizes/spacing * chore(desktop): biome format InlineCommentComposer (drive-by, unblocks pre-push) * [Claude Design adoption] PR-B: examples gallery (#50) * feat(hub): examples gallery as first-class section PR-B from doc 28 (Claude Design adoption). Ships eight curated examples (cosmic animation, organic loaders, landing page, case study, dashboard, pitch slide, welcome email, mobile habit tracker) with stylised inline SVG thumbnails, an `ExamplesTab` view component, and full en + zh-CN translations for every title, description, category label, and surrounding chrome. The tab is self-contained: a single `onUsePrompt` prop hands the chosen example back to the host so PR-A can wire it into the hub without further plumbing. No App.tsx changes here — independent of PR-A landing first. Compatibility ✅ Upgradeability ✅ No bloat ✅ Elegance ✅ Signed-off-by: hqhq1025 <[email protected]> * fix(hub): use --font-size-body-xs token for example card category badge Replaces hardcoded text-[10px] with the existing --font-size-body-xs typography token (11px). Resolves Codex token-only UI constraint blocker on PR #50. Signed-off-by: hqhq1025 <[email protected]> * fix(templates): throw on missing locale content for examples (no silent fallback) Codex blocker on PR #50: getExamples returned `{ title: id, description: '' }` when both the requested locale and the en fallback lacked an entry, shipping degraded UI without surfacing the bug. - Throw a descriptive Error when no registry has the example id - Add vitest case asserting the throw path - Drive-by: biome format apps/desktop/src/renderer/src/components/InlineCommentComposer.tsx (pre-existing format drift on main blocking pre-push) Signed-off-by: hqhq1025 <[email protected]> --------- Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): tokenize PreviewToolbar values missed by #57 (#62) * fix(desktop): tokenize remaining PreviewToolbar hardcoded values missed by #57 Replaces three remaining hardcoded px values in PreviewToolbar with design tokens to keep the export menu aligned with the token system: - h-[30px] → h-[var(--size-control-sm)] - w-[14px] h-[14px] → w-[var(--size-icon-sm)] h-[var(--size-icon-sm)] - min-w-[200px] → min-w-[var(--size-stage-min)] Adds a new --size-stage-min (200px) token to packages/ui for menu/popover minimum widths. The other three font-size values noted in the original audit were already tokenized by #57. * chore: format Download icon and silence pre-existing core complexity lint - Wrap PreviewToolbar Download icon across multiple lines per Biome formatter - Add biome-ignore for pre-existing noExcessiveCognitiveComplexity in packages/core/src/index.ts runModel (blocks pre-push hook; refactor tracked separately, mirrors the precedent set in 9051fae) * fix(desktop): drop unused fileURLToPath import in main entry (#63) * fix(desktop): PreviewPane hint pill + iframe respect dark mode (#69) Co-authored-by: Claude <[email protected]> * fix(desktop): i18n ThemeToggle aria/tooltip strings (#70) Replace hardcoded English with t() calls; add theme.{toggleAria,switchToLight,switchToDark} to en.json and zh-CN.json. Co-authored-by: Claude <[email protected]> * fix(desktop): tokenize raw utilities in preview Loading/Error states (#64) * fix(desktop): tokenize raw utilities in preview Loading/Error states Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): tokenize space-y-3 in LoadingState skeleton header Signed-off-by: hqhq1025 <[email protected]> --------- Signed-off-by: hqhq1025 <[email protected]> * feat(desktop): Snapshots SQLite schema + IPC handlers (PR-A) (#29) * feat(desktop): snapshots SQLite + IPC foundation (PR-A of version history) - packages/shared: DesignSnapshotV1 + DesignV1 Zod schemas, SnapshotCreateInput interface - apps/desktop: better-sqlite3 persistence (designs + design_snapshots tables, WAL, FK cascade) initSnapshotsDb(path) for production, initInMemoryDb() for tests - snapshots:v1:* IPC handlers: list-designs, create-design, list, get, create, delete All reject malformed payloads with CodesignError('IPC_BAD_INPUT') - preload: window.codesign.snapshots namespace bridged to renderer - Vitest: 26 new tests across snapshots-db + snapshots-ipc (111 pass total) New dep: better-sqlite3@^11 (MIT, native, already in CLAUDE.md stack) No ulid added — using crypto.randomUUID() instead. PR-B will wire auto-snapshot-on-sendPrompt + history sidebar UI. Signed-off-by: hqhq1025 <[email protected]> * style(desktop): fix biome formatting in snapshots-db.test.ts Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): address codex snapshots cascade/parent/sort findings - Enable PRAGMA foreign_keys = ON in applySchema so ON DELETE CASCADE / SET NULL fire in production (previously only the test enabled it manually, hiding the regression). - Constrain design_snapshots.parent_id with a self-referential FK (ON DELETE SET NULL) and validate in the IPC layer that parentId resolves to a snapshot in the same design — prevents silent history corruption from stale or cross-design ids. - Sort listDesigns by updated_at DESC, created_at DESC so designs bubble after new snapshots are added (createSnapshot already bumps updated_at). - Drop the now-redundant manual pragma in the cascade test and add coverage for parent SET NULL, cross-design parent rejection, and the activity-based design sort. Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): reject invalid create-design input instead of silent default The snapshots:v1:create-design IPC handler coerced empty/non-string payloads to 'Untitled design', hiding caller bugs and violating the no-silent-fallback rule. Reject with IPC_BAD_INPUT instead, drop the matching preload coercion, and update tests to cover the new contract. Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): translate SQLite errors to typed IPC contract in snapshots-ipc Wraps every snapshot DB call in a translateSqliteError helper that maps better-sqlite3 SqliteError codes to typed CodesignError instances: - SQLITE_CONSTRAINT_FOREIGNKEY -> IPC_BAD_INPUT - SQLITE_BUSY / SQLITE_LOCKED -> IPC_DB_BUSY - SQLITE_FULL -> IPC_DB_FULL - other -> IPC_DB_ERROR (full details logged server-side) Renderer no longer sees raw provider error strings. Adds vitest cases that stub better-sqlite3 prepare() to throw each code and assert the right CodesignError is surfaced. Signed-off-by: hqhq1025 <[email protected]> * fix(snapshots-ipc): map SQLite constraint subcodes individually Bare SQLITE_CONSTRAINT also covers UNIQUE / NOT NULL / CHECK violations, so translating it as "Parent snapshot does not exist" misled the UI on unrelated failures. Match each subcode explicitly: - SQLITE_CONSTRAINT_FOREIGNKEY -> IPC_BAD_INPUT, parent-snapshot message - SQLITE_CONSTRAINT_UNIQUE/PK -> IPC_CONFLICT, "Snapshot already exists" - SQLITE_CONSTRAINT_NOTNULL/CHECK -> IPC_BAD_INPUT, neutral constraint message - bare SQLITE_CONSTRAINT (no suffix) -> generic IPC_DB_ERROR Adds vitest coverage for each new branch. Signed-off-by: hqhq1025 <[email protected]> * fix(snapshots-ipc): clarify FK error covers design and parent SQLITE_CONSTRAINT_FOREIGNKEY fires for both a missing design_id and a missing parent_id, but the previous translation always reported "Parent snapshot does not exist" — leading contributors to look for the wrong cause. Key the FK message by call-site context and use a message that names both columns ("Referenced design or parent snapshot does not exist"); fall back to a generic "Referenced item does not exist" for unmapped contexts. Also extracts the static SQLite-code lookup into a small helper to keep translateSqliteError below the cognitive-complexity threshold. Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): schema-version snapshots:v1 IPC payloads Every snapshots:v1:* object payload now carries `schemaVersion: 1`, both on the wire (preload bridge) and in the main-process parser. Mismatched or missing versions throw IPC_BAD_INPUT so future handler revisions can break cleanly instead of silently mis-parsing legacy callers. - snapshots-ipc.ts: requireSchemaV1() helper applied to list-designs, list, get, create, delete, create-design (now object-shaped). - preload/index.ts: every snapshots invoke wraps the payload with { schemaVersion: 1, ... }. - snapshots-ipc.test.ts: updated existing fixtures via a v1() helper and added a parameterised gating suite that asserts every channel rejects missing and mismatched schemaVersion values. Addresses Codex Major review on PR #29. Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): degrade gracefully when snapshots DB init fails Previously initSnapshotsDb() ran inside app.whenReady() without a guard, so any open/migration/native-binding failure rejected the boot promise and prevented createWindow() from ever firing — the user got a silent no-window app. Add safeInitSnapshotsDb() wrapper that captures the error, then in main boot: log it with full stack via electron-log, surface it through dialog.showErrorBox, skip registerSnapshotsIpc, and continue to open the window. Snapshot-dependent renderer features will fail loudly via their IPC channels, but the rest of the app stays usable. Also reset the singleton if applySchema throws so a retry can recover instead of returning a half-open DB handle. Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): typed SNAPSHOTS_UNAVAILABLE stub when snapshots DB init fails When safeInitSnapshotsDb fails at boot, main/index.ts previously skipped registerSnapshotsIpc entirely. Renderer calls to window.codesign.snapshots.* then surfaced as Electron's opaque "No handler registered" rejection, violating the no-silent-fallback / error-context requirement (PR #29 codex Major). Install registerSnapshotsUnavailableIpc stubs for the same channel set so every renderer call rejects with a typed CodesignError carrying code SNAPSHOTS_UNAVAILABLE and a message pointing the user at Settings → Storage. The channel list is exported (SNAPSHOTS_CHANNELS_V1) so the test can pin it to the live registration set and prevent drift. Adds vitest coverage that asserts SNAPSHOTS_UNAVAILABLE is thrown for every channel and that the stub set matches registerSnapshotsIpc exactly. Signed-off-by: hqhq1025 <[email protected]> --------- Signed-off-by: hqhq1025 <[email protected]> * feat(exporters): Markdown export of generated artifact (#66) * feat(exporters): add Markdown export with simple HTML→MD conversion Adds a tier-1 Markdown exporter (.md with YAML frontmatter carrying schemaVersion: 1). Conversion is a small set of regex passes covering h1..h6, p, a, img, ul/ol, strong/em, code/pre — anything else is stripped. Zero new runtime deps. Wired through the existing exporter IPC + Preview toolbar Export menu, with en + zh-CN i18n strings. Compatibility ✅ Upgradeability ✅ No bloat ✅ Elegance ✅ Signed-off-by: hqhq1025 <[email protected]> * fix(exporters): allowlist URL schemes in markdown export Sanitize <a href> and <img src> during htmlToMarkdown so unsafe schemes (javascript:, vbscript:, file:, non-image data:, etc.) cannot ride into the exported .md and execute via downstream renderers. Allow http(s), mailto, relative URLs, fragments; permit data:image/* only on <img>. Unsafe links collapse to plain text, unsafe images are dropped. Also fix the IPC default extension for the markdown format (`design.md` instead of `design.markdown`) when no defaultFilename is provided. Addresses codex review on PR #66. Signed-off-by: hqhq1025 <[email protected]> * fix(exporters): close encoded-scheme bypass in markdown sanitizeUrl Decode HTML entities (named, hex, decimal), URL %escapes, and strip control characters (TAB/CR/LF/etc.) before scheme allowlisting so payloads like javascript:, %6Aavascript:, JavaScript:, and tab- prefixed schemes can no longer slip through the link/image sanitizer. Adds regression tests covering each bypass vector. Signed-off-by: hqhq1025 <[email protected]> * fix(exporters): only decode scheme prefix in sanitizeUrl The previous follow-up ran decodeURIComponent on the entire URL which both threw on legitimate inputs containing literal `%` (dropping the link entirely) and rewrote percent-escaped path/query characters such as %2F or %C3%A9. Restrict entity + percent decoding to the scheme portion (before the first colon) used solely for the safety check, and emit the original (control-stripped, trimmed) URL when the scheme is allowlisted. Adds regression tests for %2F in query, UTF-8 percent-escapes in path, literal trailing %, and confirms the existing entity / %-encoded javascript bypass guards still strip dangerous schemes. Signed-off-by: hqhq1025 <[email protected]> --------- Signed-off-by: hqhq1025 <[email protected]> * [Claude Design adoption] PR-A: designs hub + multi-type create wizard (#51) * feat(desktop): designs hub + multi-type create wizard (Claude Design adoption PR-A) Replaces the straight-to-composer launch flow with a designs hub modeled on Claude Design's Recent / Your designs / Examples / Design systems navigation plus a typed create wizard (Prototype / Slide deck / From template / Other). - Hub view with four sibling tabs and a primary "New design" CTA - Modal create flow; CTA stays disabled until a project name is provided - Per-type forms; PR-F will fill in the wireframe vs high-fidelity cards - Examples and Design systems tabs ship as placeholders for PR-B / PR-C - Project schema lives in shared with schemaVersion=1; persisted to localStorage for now (SQLite migration arrives with PR-C) - Full en + zh-CN coverage; tokens from packages/ui (no hardcoded values) - Vitest covering the create-draft logic; existing 144-test suite untouched Compatibility: green - no IPC/main changes, no schema breaks. Upgradeability: green - schemaVersion field on every Project payload. No bloat: green - no new dependencies; reuses lucide-react + zustand. Elegance: green - single store action per intent; per-type forms < 40 LOC. Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): surface project storage errors via toast + warn (no silent fallback) - readStoredProjects/persistProjects now console.warn and return an error string instead of swallowing exceptions; createProject pushes an error toast on persist failure while keeping in-memory state consistent. - Validate stored projects with the Project zod schema (safeParse) and count rejected records so corrupted entries surface a toast instead of silently disappearing. - openProject resets project-scoped workspace state (messages, preview, inputs, generation flags) to prevent cross-project state leakage. - Add errors.projectStorageFailed i18n key (en + zh-CN). - Vitest: mock localStorage.setItem to throw, assert toast pushed and the new project is still added to in-memory state. Addresses Codex blocker on PR #51. Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): replace hardcoded checkbox sizing with --size-icon-md token SlideDeckForm checkbox used `w-4 h-4` literals which violate the token-only UI constraint. Swap to `var(--size-icon-md)` (16px) so the control scales with centralized theming. Addresses Codex blocker on PR #51. Signed-off-by: hqhq1025 <[email protected]> * chore: clear pre-existing biome errors blocking pre-push hook - tailwindExtractor: replace non-null assertion with safe cast - InlineCommentComposer: apply formatter Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): wire ExamplesTab onUsePrompt after rebase onto PR-B PR-B merged ExamplesTab as a real component requiring an onUsePrompt callback. After rebase, surface that prop through HubView and have App prefill the workspace prompt + switch view. Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): reset project-scoped workspace state in createProject createProject was only clearing messages/previewHtml/generationStage, leaving inputFiles, referenceUrl, selectedElement, lastPromptInput, and generation/error flags inherited from the previously open project. Mirror openProject's full reset so a freshly created project starts with a clean workspace and prompt context. Adds vitest coverage for the reset. Refs codex review on PR #51. Signed-off-by: hqhq1025 <[email protected]> * fix(ui): replace hardcoded sizing in hub/create with new tokens PR-A introduced raw arbitrary-value Tailwind classes (360px sidebar, 560px modal, 60ch prose, 240px card minimum, 1px hover lift) that violate the token-only constraint in CLAUDE.md. Add five tokens to packages/ui/src/tokens.css and route every call site through them so themes and density tweaks stay centralized. --------- Signed-off-by: hqhq1025 <[email protected]> * feat(core): Skills system foundation — loader + 4 starter skills (#33) * feat(core): skills loader + 4 starter skills + provider injector (PR-A) - packages/shared/src/skills.ts: SkillFrontmatterV1 zod schema + LoadedSkill interface (canonical location; avoids core→providers circular dep) - packages/core/src/skills/: inline YAML frontmatter parser, loadSkillsFromDir, loadAllSkills with 3-tier priority (project > user > builtin), loader tests - packages/core/src/skills/builtin/: 4 starter skills (frontend-design-anti-slop, pitch-deck, data-viz-recharts, mobile-mock) — self-written, Apache-2.0, no Anthropic SKILL.md text copied - packages/providers/src/skill-injector.ts: injectSkillsIntoMessages, pure, supports system and prefix scope, provider-wildcard matching, injector tests No new runtime deps added. Inline YAML parser (~100 lines) handles folded scalars, nested mappings, inline + block sequences. Signed-off-by: hqhq1025 <[email protected]> * fix(core): skills loader collects errors and throws instead of silent drop Replace the three silent continue/console.warn paths in loadSkillsFromDir with error collection; after processing all files, throw CodesignError 'SKILL_LOAD_FAILED' if any errors were accumulated. Update tests to expect the throw, and add a new loadAllSkills test for a broken skill (missing description field). Signed-off-by: hqhq1025 <[email protected]> * fix(core): propagate non-ENOENT errors in skills loader Signed-off-by: hqhq1025 <[email protected]> * fix(providers): sort skills into canonical order before injection Mixed-scope skill injection was order-dependent: whichever skill appeared first in the caller's array decided whether the block went into the system prompt or the first user message. That made prompt-shaping behaviour a function of loader iteration order rather than the active skill set. Sort skills by source precedence (project > user > builtin) and then alphabetical name before building the block. The chosen scope, the concatenated body, and the resulting prompt blob are now byte-identical across runs regardless of input order, which also stabilises prompt caching and snapshot tests. Adds a vitest case that feeds three random permutations of a five-skill fixture and asserts the resulting system content is byte-identical to the canonical sort, plus a mixed system/prefix scope test that confirms the higher-precedence skill picks the channel. Signed-off-by: hqhq1025 <[email protected]> * fix(providers): inject mixed-scope skills into separate channels Previously a mixed system/prefix skill set was concatenated into a single block and injected via the highest-precedence skill's channel, silently routing the rest into the wrong channel and violating each skill's trigger.scope contract. Now we partition active skills by scope and inject system-scope skills into the system message and prefix-scope skills into the first user message, preserving canonical order within each channel. Signed-off-by: hqhq1025 <[email protected]> * fix(core): preserve newlines in YAML literal block scalar (PR #33 codex Minor) --------- Signed-off-by: hqhq1025 <[email protected]> * feat(desktop): viewport switcher (desktop / tablet / mobile with phone bezel) (#32) * feat(desktop): mobile/tablet/desktop viewport preview with phone bezel - Add PreviewViewport type and previewViewport/setPreviewViewport to store - Add PhoneFrame component (CSS-only iPhone bezel, fully tokenised) - Add viewport switcher (Desktop/Tablet/Mobile) to PreviewToolbar - PreviewPane renders iframe inside PhoneFrame at 375x812 for mobile, centred 768px container for tablet, and full-width for desktop - Add preview.viewport i18n keys (en + zh-CN) - Add 4 unit tests for setPreviewViewport in store.test.ts Signed-off-by: hqhq1025 <[email protected]> * fix(ui): tokenize phone frame dimensions + give tablet wrapper a height Replace hard-coded px values in PhoneFrame with CSS custom properties from packages/ui tokens.css. Add --size-preview-mobile-*, --size-preview-tablet-width, --radius-phone, and --border-width-strong tokens. Fix tablet branch in PreviewPane to propagate h-full so the iframe is no longer zero-height. Signed-off-by: hqhq1025 <[email protected]> * fix(ui): tokenize preview viewport sizes (mobile/tablet/desktop) Signed-off-by: hqhq1025 <[email protected]> * fix(ui): tokenize preview hint badge color/size Address Codex blocker re-flagged on PR #32: - Replace hardcoded values in PreviewPane hint badge (left-5/top-5, bg-[rgba(255,255,255,0.88)], px-3/py-1, text-[11px]) with --space-5/--space-3/--space-1, --color-surface-elevated, --text-xs. - Replace hardcoded sizes/motion in PreviewToolbar viewport controls and Download button (w-[14px], w-[30px], h-[30px], duration-150, literal cubic-bezier) with --size-icon-sm, --size-control-xs, --duration-fast, --ease-out. - Add tokens: --color-surface-elevated (light + dark), --size-control-xs (30px), --size-icon-sm (14px). Signed-off-by: hqhq1025 <[email protected]> * fix(ui): tokenize PhoneFrame border + remaining hardcoded values Replace hard-coded `outline: '1px solid ...'` with the existing `--shadow-inset-soft` token composed into boxShadow. Removes the last non-tokenized value in PhoneFrame so the component is fully driven by packages/ui tokens. Signed-off-by: hqhq1025 <[email protected]> * fix(ui): make PhoneFrame notch overlay click-through (pointer-events: none) Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): add aria-label to viewport switcher buttons title alone is not a reliable accessible name for screen readers; add aria-label using the same i18n string so assistive tech announces Mobile/Tablet/Desktop instead of a generic button. Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): tokenize iframe background in mobile preview (PR #32 codex Major) * fix(desktop): tokenize desktop preview iframe bg + dedup icon size tokens (PR #32 codex follow-up) --------- Signed-off-by: hqhq1025 <[email protected]> * fix(core): exclude craft-directives from tweak mode (PR #43 codex Major) --------- Signed-off-by: hqhq1025 <[email protected]> Co-authored-by: Claude <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PhoneFramecomponent (iPhone bezel with notch, 375 × 812) — no new dependencyPreviewViewporttype +previewViewport/setPreviewViewportadded to Zustand storepreview.viewport.{label,desktop,tablet,mobile}in bothenandzh-CNDemo coverage
Enables reproduction of Claude Design's Calm Spaces meditation app demo (mobile mock).
Test plan
store.test.ts: 4 new tests coversetPreviewViewportdefault + all three transitionspnpm -r typecheck && pnpm lint && pnpm test— all greenCompatibility / upgradeability / no bloat / elegance
PhoneFrameis ~40 lines of CSSvar(--*)tokens; no hard-coded px/colours in component code