Skip to content

getCSS - first implementation#7

Open
ameerabuf wants to merge 28 commits intomasterfrom
get_css
Open

getCSS - first implementation#7
ameerabuf wants to merge 28 commits intomasterfrom
get_css

Conversation

@ameerabuf
Copy link
Contributor

No description provided.

@ameerabuf ameerabuf requested a review from ydaniv January 11, 2026 22:32
return fullEffect;
}

return null;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case of else here we probably want to generate an override declaration that removes the animation/transition/state.
This is current implementation in TB. An empty animation is an override of none.
We probably won't declare this openly in the docs, but we still need to support this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how do you override a transition? how can you know which props to override and with what values? that seems problematic... with animation you can do animation: none under the correct selector, but I am not sure how to handle transitions

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the Viewer's method sucks, but we need to support this flow.
I guess with transitions they blocked it on the JS level.


for (const effectRef of interaction.effects) {
const effect = resolveEffect(effectRef, config.effects, interaction, configConditions);
if (!effect) continue;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll probably won't have a case of !effect, and instead override with none.

Comment on lines +299 to +300
const escapedKey = CSS.escape(effect.key);
const keyWithNoSpecialChars = effect.key.replace(/[^\w-]/g, '');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In SSR you might not have the actual key, and instead have a key template, i.e. with [] in it.
We need to consider these cases and solve them somehow.
Perhaps if not possible, we could filter these in the beginning and only render here what we can.
Either we skip those and fallback to initial with WAAPI, or render those separately.
Basically you could have template keys in the Config, and the full keys are rendered to the HTML/DOM.
If these are known at SSR they can probably be rendered on SSR, perhaps in a dedicated call to generateCSS().
If not known at SSR, they're probably better off with a fallback to WAAPI.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the key is given as a template does it mean that all targets that match the template get the animation? do we only have a single [] inside? can we maybe use the start before [ and the end after ] to create a selector that matches all matched keys? a bit flakey I guess..

Copy link
Contributor Author

@ameerabuf ameerabuf Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway it is only the selector that is problematic - keyWithNoSpecialChars is only used to generate a custom-property name that will not leak by inheritance. excapedKey is used for the selectors and we might be able to do something like effect.key.split('[]').map((keyPart, index, arr)=>'[data-interact-key${index === arr.length - 1 ? "$" : index === 0 ? "^" : "*"}=${keyPart}]')

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the key is given as a template does it mean that all targets that match the template get the animation?

Yes.

do we only have a single [] inside?

Can have multiple.

can we maybe use the start before [ and the end after ] to create a selector that matches all matched keys?

Won't work.

The problem is that the Config has templates, not keys of instances. You don't have the data with the ids.

// SUITE 2: Trigger Filtering
// ============================================================================
describe('trigger filtering', () => {
const timeTriggers = ['viewEnter', 'hover', 'click', 'animationEnd', 'pageVisible'] as const;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to drop this one:

Suggested change
const timeTriggers = ['viewEnter', 'hover', 'click', 'animationEnd', 'pageVisible'] as const;
const timeTriggers = ['viewEnter', 'hover', 'click', 'animationEnd'] as const;

return fullEffect;
}

return null;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the Viewer's method sucks, but we need to support this flow.
I guess with transitions they blocked it on the JS level.

Comment on lines +299 to +300
const escapedKey = CSS.escape(effect.key);
const keyWithNoSpecialChars = effect.key.replace(/[^\w-]/g, '');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the key is given as a template does it mean that all targets that match the template get the animation?

Yes.

do we only have a single [] inside?

Can have multiple.

can we maybe use the start before [ and the end after ] to create a selector that matches all matched keys?

Won't work.

The problem is that the Config has templates, not keys of instances. You don't have the data with the ids.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants