-
Notifications
You must be signed in to change notification settings - Fork 563
refactor: app mode — customizable layout #11317
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
d4ea383
96330d5
0d31e3b
43302f8
7da9659
451752b
1d16073
dde524f
8f8aa62
a4addaa
e28af8c
d9ae0dd
c9b8e53
299292a
d436f54
385b611
9657403
3361a66
3b70a54
551a702
76dc717
550d855
9adcea8
916ad31
d621c7d
69e1bff
82f1980
fd022de
e8c7076
38b197c
adc1db4
fbddb28
d1f29f0
32ab581
583ba29
0628acf
7c0d078
ccf166d
4083442
f067835
fc8e2f3
09d6ff6
8f3b9be
d2e637b
98f57a9
8c3fb37
a4128c8
ea0601a
201fc1e
bd4f9ef
17ecf38
895c0c1
56bbd35
d8355f4
543f4b3
75b77b0
d676184
ed93de5
36c23a6
ff60b57
1289e61
d4dc570
f6a29be
c497c8c
90f140b
563742f
1d16598
61ff478
d434a0d
91bd183
b007a55
676a443
13b3e09
6f91765
2a21d12
c274133
0d18fe0
8f00acb
5edcdf8
274d32c
152653a
1f0cfea
de52814
4754980
681a959
989754e
4fb60b8
2746856
9c171c1
055e46e
2640d09
c6cf567
fd75632
1acdb42
a4d4145
1a7300b
7e5a95a
6b9dfe8
f357c57
c668618
4c140cb
1a1025b
c48d731
5a31405
2f7e90a
7a5e375
c997195
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
PanelBlockList: - Drop :input(:not(textarea)) tautology; <textarea> isn't an <input>. - Stabilize row v-for key (rowIdx) so in-row reorders don't remount. - Replace inline FLIP block with useFlipReorder() — Web Animations API, no double-RAF, no manual transition swap. - Standard scale: gap-[10px] → gap-2.5, outline-offset-[6px] → -1.5. useFlipReorder: - New composable. Captures rects in onBeforeUpdate, plays diffs back via el.animate() in onUpdated. Sub-pixel deltas skipped to avoid jiggling on unrelated reactive updates. skipKey opt-out for the dragging element so its lift treatment doesn't compete with the slide on its siblings. panelPresetClasses: - Trim 60 → 28 lines. Constant names speak for themselves; one short doc note kept for the non-obvious sidebar-offset gotcha. Panel layout persistence: - New fields under extra.linearData.layout: panelPreset, panelCollapsed, panelWidthCells, panelRows. Loaded in loadSelections; written from the existing builder-mode watch (block reorder is builder-only). - Schema + LinearLayoutConfig type extended. - Drop the "in-memory only" comment from panelTypes.ts.
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| import { onBeforeUpdate, onUpdated } from 'vue' | ||
| import type { Ref } from 'vue' | ||
|
|
||
| interface FlipReorderOptions { | ||
| /** Animation duration in ms. Default 200. */ | ||
| durationMs?: number | ||
| /** Returns the flip-key of an element to skip — used to keep a | ||
| * drag/lift treatment in place while siblings animate. */ | ||
| skipKey?: () => string | null | ||
| } | ||
|
|
||
| /** | ||
| * FLIP-animates `[data-flip-key]` descendants of `containerEl` whose | ||
| * positions changed between Vue patches. | ||
| * | ||
| * Captures rects in `onBeforeUpdate`, then in `onUpdated` plays the | ||
| * difference back via the Web Animations API. Sub-pixel deltas are | ||
| * skipped so unrelated reactive updates don't jiggle the layout. | ||
| */ | ||
| export function useFlipReorder( | ||
| containerEl: Ref<HTMLElement | null>, | ||
| options: FlipReorderOptions = {} | ||
| ): void { | ||
| const { durationMs = 200, skipKey } = options | ||
| const prevRects = new Map<string, DOMRect>() | ||
|
|
||
| onBeforeUpdate(() => { | ||
| prevRects.clear() | ||
| const els = | ||
| containerEl.value?.querySelectorAll<HTMLElement>('[data-flip-key]') | ||
| if (!els) return | ||
| for (const el of els) { | ||
| const key = el.dataset.flipKey | ||
| if (key) prevRects.set(key, el.getBoundingClientRect()) | ||
| } | ||
| }) | ||
|
|
||
| onUpdated(() => { | ||
| const skip = skipKey?.() ?? null | ||
| const els = | ||
| containerEl.value?.querySelectorAll<HTMLElement>('[data-flip-key]') | ||
| if (!els) return | ||
| for (const el of els) { | ||
| const key = el.dataset.flipKey | ||
| if (!key || key === skip) continue | ||
| const prev = prevRects.get(key) | ||
| if (!prev) continue | ||
| const next = el.getBoundingClientRect() | ||
| const dx = prev.left - next.left | ||
| const dy = prev.top - next.top | ||
| // Sub-pixel deltas would jiggle on every reactive update. | ||
| if (Math.abs(dx) < 0.5 && Math.abs(dy) < 0.5) continue | ||
| el.animate( | ||
| [{ transform: `translate(${dx}px, ${dy}px)` }, { transform: 'none' }], | ||
| { duration: durationMs, easing: 'ease' } | ||
| ) | ||
| } | ||
|
Comment on lines
+33
to
+52
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Honor reduced-motion when replaying FLIP animations. This path animates every reorder via WAAPI, so users who have 🤖 Prompt for AI Agents
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Deferred —
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The PR description images show:
If helpful later, here’s the minimal composable change that keeps behavior identical but skips WAAPI when users prefer reduced motion. It doesn’t alter panel UX; it only gates the animate() call. // src/components/appMode/layout/panels/useFlipReorder.ts
function prefersReducedMotion(): boolean {
try {
return typeof window !== 'undefined' &&
typeof window.matchMedia === 'function' &&
window.matchMedia('(prefers-reduced-motion: reduce)').matches
} catch {
return false
}
}
onBeforeUpdate(() => {
prevRects.clear()
if (prefersReducedMotion()) return
const els = containerEl.value?.querySelectorAll<HTMLElement>('[data-flip-key]')
if (!els) return
for (const el of els) {
const key = el.dataset.flipKey
if (key) prevRects.set(key, el.getBoundingClientRect())
}
})
onUpdated(() => {
if (prefersReducedMotion()) return // instant snap: no WAAPI
const skip = skipKey?.() ?? null
const els = containerEl.value?.querySelectorAll<HTMLElement>('[data-flip-key]')
if (!els) return
for (const el of els) {
const key = el.dataset.flipKey
if (!key || key === skip) continue
const prev = prevRects.get(key)
if (!prev) continue
const next = el.getBoundingClientRect()
const dx = prev.left - next.left
const dy = prev.top - next.top
if (Math.abs(dx) < 0.5 && Math.abs(dy) < 0.5) continue
el.animate(
[{ transform: `translate(${dx}px, ${dy}px)` }, { transform: 'none' }],
{ duration: durationMs, easing: 'ease' }
)
}
})Want me to open a follow-up GitHub issue now with:
✏️ Learnings added
🧠 Learnings used |
||
| }) | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n src/components/appMode/layout/panels/panelPresetClasses.ts | head -30Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1590
🌐 Web query:
Tailwind CSS arbitrary values spaces underscores calc function💡 Result:
In Tailwind CSS, arbitrary values use square bracket notation like w-[200px] to apply any CSS value directly.[1] Spaces are not allowed in class names because browsers split classes on spaces, so they must be replaced with underscores () which Tailwind automatically converts to spaces at build time.[1][2] For example, grid-cols-[1fr_500px_2fr] becomes grid-cols: 1fr 500px 2fr in CSS.[1] This applies specifically to the calc function in arbitrary values. Spaces inside calc must be replaced with underscores: w-[calc(100%-2rem)] generates width: calc(100% - 2rem);[3][1][4][5] Spaces like w-[calc(100% - 2rem)] break because the browser parses it as multiple classes.[6][7] Underscores are converted to spaces unless in contexts like URLs (where they are preserved), the first argument of var or theme, or explicitly escaped with backslash (\) to keep the underscore.[1][8][2] For calc, use underscores for operator spacing: h-[calc(100vh_-64px)].[3] This behavior is unchanged in Tailwind CSS v4 (latest release v4.2.4 as of 2026-04).[9] Examples: - Valid: w-[calc(100%-20px)], p-[calc(3rem*_2)] - Invalid (use ): w-[calc(100% - 20px)] In JS/JSX, escape backslashes with String.raw
hello\\_worldto preserve \ if needed.[8]Citations:
🌐 Web query:
CSS calc() operator whitespace requirements specification💡 Result:
The CSS calc function requires whitespace on both sides of the + (addition) and - (subtraction) operators due to the formal grammar defined in the CSS Values and Units specification. This is explicitly stated in section 8.1.1 (Syntax) of CSS Values and Units Module Level 3 (and carried forward in Levels 4 and 5): "In addition, white space is required on both sides of the + and - operators. (The * and / operators can be used without white space around them.)" [1][2]. The grammar productions are: = calc( ) = [ [ '+' | '-' ] ]* = [ [ '' | '/' ] ] = | | | ( ) [1][2][3]. This requirement prevents parsing ambiguities. Without spaces, expressions like calc(50% -8px) are parsed as a percentage followed by a negative length (-8px), which is invalid, rather than subtraction [4][5][6][3]. Similarly, calc(8px + -50%) would be a length followed by a unary minus on the percentage [4][6]. The hyphen (-) can appear in CSS identifiers/dimensions, so spaces distinguish the binary operator [7][6]. In contrast, * (multiplication) and / (division) do not require whitespace, though it is recommended for consistency [4][1][2][3]. This rule remains unchanged in the latest specifications (Levels 4 and 5 as of 2024/2025 drafts) [8][9]. MDN confirms: "The + and - operators must be surrounded by whitespace" [4][10][3].
Citations:
calc()expressions are missing required whitespace around+and-operators.When an arbitrary value needs to contain a space, use an underscore (
_) instead and Tailwind will automatically convert it to a space at build-time. The CSS spec (CSS Values and Units Module) explicitly requires whitespace on both sides of+and-incalc()— omitting it produces spec-invalid CSS that creates parsing ambiguity. The correct pattern isw-[calc(100%_-_2rem)]for proper parsing.SLOT_MAX_His especially at risk:calc(100%-var(...)…)can be misread as100%followed by a unary-negated custom property, which is syntactically invalid.🛠️ Proposed fix — use `_` as space placeholder
🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deferred — out of scope. The hex/rgb values in
panelPresetClassesare deliberate visual identity (preset accents), not theme tokens. Documented in the LatentPreview comment block as the project's pattern for fixed-identity placeholder visuals.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR description images show:
@eliheuerThanks — acknowledged on the fixed-identity color accents; I won’t push theme tokens there.Separate from colors, my note on src/components/appMode/layout/panels/panelPresetClasses.ts (Line 11–18) was about calc() spacing. To avoid calc parsing issues and keep Tailwind arbitrary values valid, we should add underscores as space placeholders around + and - inside the calc() expressions. RIGHT_EDGE/LEFT_EDGE using the v4 var shorthand (right-(--spacing-layout-outer)/left-(--…)) are fine as-is.
Here’s the minimal, targeted tweak:
If you’re okay with this tiny calc fix, I’ll consider the thread closed with colors deferred.
✏️ Learnings added
🧠 Learnings used