[pull] main from Comfy-Org:main - #27
Merged
Merged
Conversation
#14847) ## Summary - guard non-finite trim/crop values and preview seek times - downscale filmstrip thumbnails to display size, sample 5 frames - capture frames via createImageBitmap and encode off the main thread - surface load errors, keep failed thumbnail slots, abort stale loads
> [!NOTE] > **#13842 builds on this.** It applies these two steps across the editor chrome (topbar, tabs, sidebar, canvas backdrop), so the tokens actually get consumers — Tailwind v4 tree-shakes `@theme` colours nothing references. Reviewing them together gives the full picture; merging this one first reduces #13842 to just the application. Adds two darker steps to the charcoal palette in packages/design-system/src/css/_palette.css: - --color-charcoal-900: #111111 - --color-charcoal-950: #09090a The scale previously stopped at 800 (#171718), so canvas surfaces relied on strokes to differentiate at the dark end. These steps extend the range for deeper canvas depth (Proposal A — Conservative, from design). Steps 100–800 are unchanged. Tailwind v4 auto-generates the bg-/text-/border-charcoal-900/950 utilities from the @theme block; no config change needed. Nothing consumes these tokens yet — a follow-up can wire semantic tokens if design wants.
## ELI5 Our warning color (gold) is bright enough to read on dark backgrounds, but on the light theme it's like yellow chalk on a whiteboard — technically there, hard to see. This adds one darker shade of the same gold to the palette so light-theme warnings can meet contrast guidelines. ## Summary - Adds `--color-gold-700: #c77700` to the design-system primitives, extending the existing gold ramp (400/500/600). - Same hue family as the current golds; measured for the light-theme `--warning-background` slot, where gold-400 sits at 1.48:1 against the hero band (WCAG wants 3:1 at that size). ## Context Identified during the mixed-severity errors panel review (#14252): dark theme measures 7.28:1 but light theme fails contrast. Agreed there that the token lands via the design-system side and #14252's follow-up points `--warning-background` at it for light theme, so this PR intentionally adds the primitive only — no consumers change and there is no visual change yet. Note Tailwind v4 may tree-shake the unused variable from built CSS until it's referenced; the source token is what the follow-up wires up. Part of DES-588. ## Test plan - [ ] `pnpm format:check` and stylelint pass (pre-commit) - [ ] No visual change expected until `--warning-background` (light) is repointed in the #14252 follow-up - [ ] Not user-visible on its own, so no feature flag needed
… ?ff= (#15033) *PR Created by the Glary-Bot Agent* --- ## Summary <img width="1672" height="941" alt="image" src="https://github.com/user-attachments/assets/9fb432ad-cdbb-4c4f-a2c5-72e723f01e88" /> Lets a Comfy employee flip a registered feature flag for their own browser tab with `?ff=name` or `?ff=name:value`, so a branch or a bug can be looked at against real cloud data without a remote-config change. ## Changes - **What**: New `src/utils/sessionFeatureFlagOverride.ts` sits at the front of the existing resolution chain in `useFeatureFlags`, which becomes: **URL override → dev localStorage → remoteConfig → server flag → default**. Overrides short-circuit the chain — nothing layers on top of them. - **Opt-in, type-aware registry.** Only flags listed in `OVERRIDABLE_FEATURE_FLAGS` can be overridden, each declaring `'boolean'` or `'string'`. The declared type is what makes `?ff=some_flag:false` resolve to boolean `false` rather than the truthy string `'false'`; string flags carry multivariate values (`?ff=signup_turnstile:enforce`). Keys are constrained to real `ServerFeatureFlag` values at compile time, so a typo in the registry fails typecheck. - **Session-scoped.** The request is captured into `sessionStorage`, so it survives reloads and in-app navigation away from the `?ff=` URL, and dies when the tab closes. A nameless `?ff=` clears the session. - **Employee-gated.** Applied only for a signed-in user with a verified `@comfy.org` address, re-checked on every read (capture happens before auth resolves, so the flag flips as soon as the user is known). Live in every environment including prod — there is no `import.meta.env.DEV` guard. - **Scoped to `.org`.** Gated on `isCloud`, so on the desktop/localhost distributions the same URL is inert and never even writes storage. - **Dependencies**: none. ## Review Focus - **Why VueFire and not `authStore`.** `authStore` imports `useFeatureFlags`, so reading auth through the store puts the whole app module graph behind every feature flag read. I tried it first and it broke `useTurnstile.test.ts` and `nodeReplacementStore.test.ts` with `api.addEventListener is not a function` — `ComfyApp` was being constructed as an import side effect. `useCurrentUser()` from `vuefire` is a leaf that works outside a component and returns the same ref `VueFireAuth` already maintains, so there is no second auth listener and no new app-graph coupling. Its `undefined`/`null`/`User` tri-state doubles as the "auth has not resolved yet" signal. - **Lookalike domains.** The check is `endsWith('@comfy.org')` on the lowercased address, so `dev@notcomfy.org` and `x@sub.comfy.org` are both rejected. Covered by tests and verified in the browser. - **Capture is idempotent, not memoised.** The query string the overrides came from is persisted next to them, so re-reading the same URL neither re-warns nor rewrites storage. Verified in the browser: exactly one console warning for an unregistered flag across app startup plus 200 extra reads. - **`getSessionOverride<T>()` keeps the caller-supplied generic** rather than a typed key→value map. Its call sites in `resolveFlag`/`resolveAuthGatedFlag` receive `flagKey: string`, so tying the return type to the key would mean threading a literal key type through all ~20 flag getters. This matches the sibling `getDevOverride<T>(flagKey: string)` exactly. Happy to tighten it if you'd rather. - Stored values are re-validated against the registry on read, so a hand-edited `sessionStorage` payload cannot introduce an unregistered key or a wrong-typed value. ## Verification `pnpm typecheck`, `pnpm lint`, `pnpm format`, `pnpm knip` and `pnpm build:cloud` all clean. Full unit suite: 15843 passed, with 2 pre-existing flaky failures in `GettingStartedScreen.test.ts` that also fail on `main` (`main` fails those plus 6 more in the same run). Manually verified in a real browser against `pnpm dev:cloud`, driving the actual module: the employee matrix below, precedence over a `ff:signup_turnstile` localStorage override, persistence across reload and SPA navigation, per-tab isolation (a second tab sees nothing), clearing via `?ff=`, and — on a second dev server running the `localhost` distribution — the same URL resolving to `undefined` with `sessionStorage` never written. ## Screenshots Each row below is a live call into the real module in the cloud build, not a mock. ## Screenshots  --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )