Uh oh!
There was an error while loading. Please reload this page.
Introduce a customizable loading stagger animation on hypertableV2 - #275
Conversation
| background-color: white; | ||
| } | ||
| // Apply stacking fixes only on cells that actually get the custom effect class. |
There was a problem hiding this comment.
left this here for context, it's in the dummy app so 🤷
Can also remove if needed 😇
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| get disableInitialRowsAnimationExtraEffectOnSelectionCells(): boolean { | ||
| return !this.initialRowsAnimation?.includeSelectionColumnInExtraEffect; | ||
| } |
There was a problem hiding this comment.
the default seems to be false, so maybe this should be the contrary ? (make it special when it's enabled ?)
There was a problem hiding this comment.
applied a change, maybe I went too far in the inversion of logic ^^ let me know :P
| } as const satisfies Omit<InitialLoadAnimationConfig, 'extraColumnCellEffectClass' | 'columns'>; | ||
| } as const satisfies Omit<InitialLoadAnimationConfig, 'extraColumnEffect' | 'includeSelectionColumnInExtraEffect'> & { | ||
| extraColumnEffect: Pick<InitialLoadAnimationExtraColumnEffect, 'delayMs'>; | ||
| includeSelectionColumnInExtraEffect: boolean; |
There was a problem hiding this comment.
nit: I would probably have picked (Pick<>) but no matter, this type is already getting out of control 😅
| if (!option) return null; | ||
| const options: InitialLoadAnimationOption = option === true ? {} : option; |
There was a problem hiding this comment.
question: The typing here threw me a little. Shouldn't it be Partial<InitialLoadAnimationConfig> like the option?
Also naming... option => options: I find it a little easy to confuse the two. Maybe a more evocative name, wdyt?
There was a problem hiding this comment.
I have removed the typing here so that is is infered directly
| ...options, | ||
| extraColumnEffect: { | ||
| ...DEFAULT_INITIAL_LOAD_ANIMATION_CONFIG.extraColumnEffect, | ||
| ...options.extraColumnEffect |
There was a problem hiding this comment.
issue: ...undefined is going to throw I think.
| ...options.extraColumnEffect | |
| ...(options?.extraColumnEffect??{}) |
There was a problem hiding this comment.
undefined case is already handled & early returned on line 141
| this.initialLoadAnimationActive = true; | ||
| const rowsAnimationWindowMs = Math.max(this.args.handler.rows.length - 1, 0) * this.initialLoadAnimation.staggerMs; | ||
| const activeDurationMs = |
There was a problem hiding this comment.
AIssue: Keep the animation context active through a configured extra-effect delay.
activeDurationMs omits extraColumnEffect.delayMs, while each cell schedules that class after the row delay plus that extra delay. A valid configuration such as maxAnimationDurationMs: 100 with extraColumnEffect.delayMs: 500 disables the context after 100ms, cancels the cell timer in resetExtraEffectState(), and the requested class is never applied. Include the maximum extra-effect delay in the active duration, or explicitly validate/document that maxAnimationDurationMs must exceed it. Add an integration test that asserts a delayed extra class actually appears.
There was a problem hiding this comment.
yup, that is wanted on my part.maxAnimationDurationMs is an overall stop including the extraEffects
What does this PR do?
This PR adds an initial-load animation system to HyperTableV2:
OptionSetwith a newinitialRowsAnimationconfiguration objectDocumentation has been updated, you can check there to have more details of how it's used.
Good PR checklist