Splittext integration#260
Conversation
ydaniv
left a comment
ydaniv
left a comment
There was a problem hiding this comment.
The whole idea of pluggable resolvers, or plugins, is that the core is not aware of them and doesn't implement anything specific to them.
It should only iterate registered plugins on init or maybe on add, and then apply the plugins' transformers on the DOM.
| // split the heading into characters on connect … | ||
| splitText: { container: '.headline', type: 'chars', hide: true }, | ||
| // … then stagger an entrance across the generated spans | ||
| effects: [{ key: 'hero', selector: '.split-c', effectId: 'fade-in' }], |
There was a problem hiding this comment.
| effects: [{ key: 'hero', selector: '.split-c', effectId: 'fade-in' }], | |
| sequences: [{ | |
| offset: 80, | |
| effects: [{ selector: '.split-c', effectId: 'fade-in' }], | |
| }, |
| ``` | ||
|
|
||
| `hide: true` opts into the FOUC guard: the container is hidden (via the | ||
| `data-text-split` attribute + a generated CSS rule) until splitting completes. |
There was a problem hiding this comment.
| `data-text-split` attribute + a generated CSS rule) until splitting completes. | |
| `data-split-text` attribute + a generated CSS rule) until splitting completes. |
|
|
||
| ## Use with @wix/interact | ||
|
|
||
| The `@wix/splittext/interact` entry point exposes a resolver that lets you wire |
There was a problem hiding this comment.
Perhaps it should say an "Interact plugin"? "resolver" is somewhat technical.
| */ | ||
| export const splitTextResolver: SplitTextResolver = { | ||
| resolve(root, config, context) { | ||
| const container = root.querySelector(config.container) as HTMLElement | null; |
There was a problem hiding this comment.
Should we allow querySelectorAll()? Almost all other selectors do. Is there a problem allowing that?
| type: config.type, | ||
| wrapperClass: config.wrapperClass, | ||
| wrapperStyle: config.wrapperStyle as Partial<CSSStyleDeclaration> | undefined, | ||
| wrapperAttrs: config.wrapperAttrs, | ||
| autoSplit: config.autoSplit, | ||
| aria: config.aria, |
There was a problem hiding this comment.
Why not simply:
| type: config.type, | |
| wrapperClass: config.wrapperClass, | |
| wrapperStyle: config.wrapperStyle as Partial<CSSStyleDeclaration> | undefined, | |
| wrapperAttrs: config.wrapperAttrs, | |
| autoSplit: config.autoSplit, | |
| aria: config.aria, | |
| ...config, |
?
| import { getInterpolatedKey } from './utilities'; | ||
| import { effectToAnimationOptions } from '../handlers/utilities'; | ||
| import { Interact, getSelector } from './Interact'; | ||
| import { applySplitText } from './splitText'; |
There was a problem hiding this comment.
the core should be agnostic to specific plugins.
| * are intentionally NOT cleared by `Interact.destroy()`, mirroring how | ||
| * `registerEffects` registrations persist across configs. | ||
| */ | ||
| private static resolvers = new Map<string, unknown>(); |
There was a problem hiding this comment.
| private static resolvers = new Map<string, unknown>(); | |
| private static plugins = new Map<string, unknown>(); |
?
| effects: {}, | ||
| sequences: {}, | ||
| conditions: {}, | ||
| splitText: {}, |
There was a problem hiding this comment.
The core should not be aware of the plugins. It could be extended dynamically with the keys of resolvers (or plugins if we prefer)
|
|
||
| function parseConfig(config: InteractConfig, useCustomElement: boolean = false): InteractCache { | ||
| const { effects: effectMap = {}, sequences: sequenceMap = {}, conditions = {} } = config; | ||
| const { |
There was a problem hiding this comment.
parsing should take care of also parsing registered plugins.
| this.listInteractionsCache = {}; | ||
| this.controllers.clear(); | ||
| this.dataCache = { effects: {}, sequences: {}, conditions: {}, interactions: {} }; | ||
| this.dataCache = { |
There was a problem hiding this comment.
Probably inside init() after parsing we want to check plugins and apply them. But there should be a generic logic that does that, nothing specific to splitText.
ydaniv
left a comment
ydaniv
left a comment
There was a problem hiding this comment.
We could revisit the plan first, correct all the issues found below inside the plan, review again, and then we could re-implement from scratch on the corrected plan, because it took a direction that is completely off course.
|
|
||
| --- | ||
|
|
||
| ## 6. Split execution hook — `src/core/add.ts` |
There was a problem hiding this comment.
This part is where it got it wrong
|
|
||
| --- | ||
|
|
||
| ## 7. New module — `src/core/splitText.ts` |
There was a problem hiding this comment.
This part is also in the wrong direction
2 participants
Description
Related Issue
Checklist
Screenshots / Demos
Additional Notes