Repository navigation
Conversation
STU-368. Which packages the Flutter SDK supplies was decided by a hand-maintained name list kept in three places, and flutter_web_plugins was in none of them. Every project declaring it - which is every web-enabled project with a plugin - read it as a dependency that could not be reproduced, and the deploy was refused for a package most of its classes never imported. The project's own pubspec already says where each package comes from; the parser read those lines and threw them away. It now returns them as sourceKey/sourceValue, so the classification is exact and cannot fall behind the SDK: sdk: -> the Flutter SDK, reproduced from the name a nested version: -> a constraint written on its own line git: / path: / nested hosted: -> not expressible in the manifest SDK_DEPENDENCIES is gone. The runner's allowedSdkPackages is gone too - it independently refused the same deploy with 'Unsupported SDK package'. That allowlist was also the only validation of an sdkPackages entry, so the package name pattern now does that job; without the swap a caller-supplied name would reach the generated pubspec unchecked. The runner still writes 'sdk: flutter' itself, so that value never comes from a caller. Refusal is now scoped per class, to that class's own imports, so one git: entry no longer leaves an entire deploy uncompiled. The accept/warn split already existed and is unchanged: a real analyzer error refuses with 422, and a class that could not be compiled is pushed with its warning logged before the provisioning request. Corrects DEPLOYMENT.md, which still described the scaffolding-closure compilation that reverting a1238f0 removed, and still claimed that nothing unverified is deployed. Verified: 274/274 tests, deploy guard 9/9, proxies 30/30, build clean, dart analyze on the runner reports no issues.
- Add src/designTokens.css as the single shipping token stylesheet, extracted from the canonical custom-code-connect-hero.html :root token set. - Link the stylesheet from index.html; reuse existing /fonts/Delight-VF.ttf and DM Mono Google Fonts path. - Replace the legacy inline :root in index.html with only the layout constant --sidebar-w, preserved until its owning shell change. - Migrate legacy variable references to new tokens: --main-bg -> --bg, --navy -> --fg, --label -> --muted, --sublabel -> --muted-2, --input-bg -> --surface-sunken, --input-border -> --border, --badge-bg -> --accent-soft, --badge-text -> --accent, --glow-color -> --accent-soft. - Centralize :focus-visible and prefers-reduced-motion defaults in the token stylesheet and remove redundant local reduced-motion rules. - No generation/deploy code changed; app.js has no affected CSS var refs.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…ust the first (cherry picked from commit d5a3c92)
…can reach a class transitively (cherry picked from commit 9d338ad)
…hape (cherry picked from commit ff9bfac)
(cherry picked from commit 1ed78b6)
…t HTML (cherry picked from commit ece1237)
Below 680px the topnav was hidden with display:none, and it holds the only link to the new #plans surface, so mobile users could reach just Home, Account and the credits modal — Plans required hand-editing the URL. Replace the mobile display:none with an adaptive active-view layout: the nav stays visible and wraps to its own topbar row under 680px with equal-width 44px touch targets, and --topbar-h grows to 100px so the fixed topbar keeps the viewport offset exact. No horizontal scrolling is introduced at 360px and hidden views stay out of the focus order. Extend the hero navigation suite with a 360x800 test asserting all three surfaces stay reachable with touch-sized targets and that the wrapped nav does not force horizontal scrolling. STU-375
switchView revealed the selected surface but never moved focus into it, and the view sections were not programmatically focusable, so focus stayed on the topbar trigger while the content changed underneath — keyboard and assistive-technology users were never placed in the newly selected surface. Give each view section tabindex=-1 and focus the revealed container once its is-active class lands (inside the same animation frame, so the element is visible and focusable). Focus only follows user-initiated navigation: the first hash restore on page load keeps focus wherever the browser put it, while later hash changes, popstate traversals and avatar-triggered switches move focus into the surface. Hidden views are already display:none and inert, so focus cannot land in them. Extend the hero navigation suite with assertions that initial load steals no focus, that keyboard and pointer navigation both land focus in the revealed view, and that the hidden view keeps zero focusable controls. STU-375
The workflow ran the unit tests and the production build but never executed the Playwright navigation suite or the accessibility modal suite, so regressions in responsive navigation, history, zoom, overflow and modal accessibility could merge while CI stayed green. Install Playwright Chromium once after the production build, then run npm run test:ui for the e2e navigation suite and npm run test:accessibility for the modal suite. The test:ui script invokes the Playwright runner against the checked-in config. Step naming follows the existing workflow and the pinned-action SHA and least-permission hardening are unchanged. STU-375
… scratch-graph approximation (cherry picked from commit e300b76)
Comment on lines
+217
to
+220
| const approximate = unrepresentable.map( | ||
| (name) => | ||
| `Your project declares ${name} from a source that cannot be reproduced outside it, so the pre-deploy compile check runs against a scratch package graph that omits ${name}. Classes that do not import ${name} directly are still checked, but against that reduced graph, so their transitive package resolution may differ from your project's and the check is approximate for them.`, | ||
| ); |
Contributor
There was a problem hiding this comment.
approximate is populated for every unrepresentable dependency without considering which classes are actually compiled. A deployment containing only dart:-only classes, whose checks are exact, therefore still reports “Verified approximately.” If all relevant classes are skipped, the result can also report both “Not verified” and “Verified approximately” for the same dependency. This makes the success result an inaccurate account of which checks were performed. Limit these notices to compiled classes with package: imports.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/customCodeVerification.js
Line: 217-220
Comment:
**Approximation Warning Misstates Checks**
`approximate` is populated for every unrepresentable dependency without considering which classes are actually compiled. A deployment containing only `dart:`-only classes, whose checks are exact, therefore still reports “Verified approximately.” If all relevant classes are skipped, the result can also report both “Not verified” and “Verified approximately” for the same dependency. This makes the success result an inaccurate account of which checks were performed. Limit these notices to compiled classes with `package:` imports.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Owner
Author
|
@greptileai — all findings from the previous review have been addressed (see new commits). Please re-review this PR. |
Owner
Author
|
Closing: this branch is already fully contained in |
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.
Closes STU-375.
What this PR does
Home,Account,Planssurfaces.switchViewrouter inapp.jsthat preserves back/forward/reload and auth query-token handling.New / modified files
index.html— new shell/hero/view markup and styles.app.js— router, shell UI updates, credits helpers, generation-stage wiring.e2e/stu375-hero-navigation.spec.js— Playwright tests for viewports, zoom, navigation, focus isolation, and history.playwright.config.js/package.json/package-lock.json— Playwright setup.Verification
npm test— 274/274 passing.npm run build— production bundle built successfully.npx playwright test e2e/stu375-hero-navigation.spec.js— 5/5 passing.Notes
The PR appears safe to merge; the only outstanding feedback is a non-blocking accuracy issue in dependency-verification notices.
Fix with agent prompt
Summary
This PR replaces the sidebar landing shell with a responsive hero and hash-based Home, Account, and Plans navigation, integrates accessible shared modal controls, adds browser and accessibility coverage to CI, and refines custom-code dependency verification. Greptile automatically discovered a related ticket that helped explain the purpose of this PR: provide responsive product surfaces while preserving navigation history, authentication query-token handling, keyboard focus, and session state.
Reviews (4) · Last reviewed commit: "fix: scope the verification skip to dire..."