fix(skills): mobile-mock no double phone chrome#81
Merged
Conversation
Contributor
There was a problem hiding this comment.
Findings
- [Major]
mobile-mockrules now conflict on whether to render iOS chrome — this can cause unstable generation behavior (sometimes adding/removing status bar/home indicator unpredictably), evidencepackages/core/src/skills/builtin/mobile-mock.md:19, context conflict atpackages/core/src/skills/builtin/mobile-mock.md:45.
Suggested fix:Output only screen contents by default. Do not render device frame/status-bar/home-indicator unless the user explicitly asks for phone chrome. Even when chrome is omitted, preserve safe areas in layout using `env(safe-area-inset-top)` and `env(safe-area-inset-bottom)`.
Summary
- Review mode: initial
- 1 issue found in modified content. Also,
docs/VISION.mdanddocs/PRINCIPLES.md: Not found in repo/docs, so policy cross-check was limited to available repo context.
Testing
- Not run (automation)
- Suggested coverage: add/update a prompt-level regression test asserting
mobile-mockoutput forbids status-bar/home-indicator markup by default and allows it only when explicit frame intent is present.
open-codesign Bot
|
|
||
| ### Viewport and Frame | ||
| Render mobile screens at **375×812px** (iPhone SE / standard size baseline) or **390×844px** (iPhone 14 standard) inside a device frame when showing a standalone mock. For scrollable content mocks, use `max-width: 390px; margin: 0 auto`. Do not render at desktop width and then scale down — it produces wrong touch target proportions. | ||
| Output **only the screen contents** as if filling a 375×812 viewport (or 390×844 for iPhone 14+). The hosting environment provides the device frame, status bar, and home indicator — DO NOT render them yourself. |
Contributor
There was a problem hiding this comment.
[Major] This new "host provides chrome" rule conflicts with later guidance that tells the model to show a static status bar row. Please make the conditions explicit (default no chrome; explicit user intent allows chrome) and keep only safe-area layout rules by default.
Suggested fix:
Output only screen contents by default. Do not render device frame/status-bar/home-indicator unless user explicitly asks for phone chrome.
Even when chrome is omitted, preserve safe areas via `env(safe-area-inset-top)` / `env(safe-area-inset-bottom)`.
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
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.
用户反馈:要 mobile prototype 时模型画手机壳,我们 PhoneFrame 又裹一层 → 双层套娃。改 skill 让模型只画屏幕内容。