Implement splitText#241
Conversation
ameerf-wix
left a comment
ameerf-wix
left a comment
There was a problem hiding this comment.
Went through everything but the tests. Will update after I go through them too.
| const { chars } = splitText('.headline', { type: 'chars' }); | ||
|
|
||
| // Animate with any library | ||
| animate(chars, { opacity: [0, 1], transform: ['translateY(8px)', 'translateY(0)'], stagger: 0.03 }); |
There was a problem hiding this comment.
why not show usage with interact? Also, what syntax is this? it is not standard WAAPI which should be chars.forEach((char, index) => char.animate({ opacity: [0, 1], transform: ['translateY(8px)', 'translateY(0)']}, {duration: 100, delay: index * 30}));
| ```css | ||
| .char { | ||
| opacity: 0; | ||
| animation: fadeUp 0.4s ease forwards; | ||
| animation-delay: calc(var(--char-index) * 0.04s); | ||
| } | ||
| @keyframes fadeUp { | ||
| to { | ||
| opacity: 1; | ||
| transform: translateY(0); | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
I think an interact example is called for
| const result = useSplitText(ref, { type: 'chars' }); | ||
|
|
||
| useEffect(() => { | ||
| if (!result) return; |
There was a problem hiding this comment.
does this suggest that useSplitText might return a nullable (on failure)? If so is if (!result) return ()=>result.revert() is needed?
There was a problem hiding this comment.
I see now that it is explained in the comment inside useSpiltText - maybe put that comment here instead of there
| | `type` | `SplitType \| SplitType[]` | — | Split eagerly on call instead of lazily | | ||
| | `wrapperClass` | `string \| WrapperClassConfig` | — | Extra CSS class(es) on wrapper spans | | ||
| | `wrapperStyle` | `Partial<CSSStyleDeclaration> \| WrapperStyleConfig` | — | Inline styles on wrapper spans | | ||
| | `wrapperAttrs` | `Record<string, string> \| WrapperAttrsConfig` | — | Custom attributes on wrapper spans | |
There was a problem hiding this comment.
this kind of types are probably good for LLM, but from a human's perspective it gives no information because I don't know what SplitType or WrapperClassConfig (etc..) are.
| | `wrapperClass` | `string \| WrapperClassConfig` | — | Extra CSS class(es) on wrapper spans | | ||
| | `wrapperStyle` | `Partial<CSSStyleDeclaration> \| WrapperStyleConfig` | — | Inline styles on wrapper spans | | ||
| | `wrapperAttrs` | `Record<string, string> \| WrapperAttrsConfig` | — | Custom attributes on wrapper spans | | ||
| | `contentAttribute` | `'none' \| 'both' \| 'attribute-only'` | `'both'` | Controls `data-content` on char/word wrappers | | ||
| | `aria` | `'auto' \| 'none'` | `'auto'` | ARIA handling mode | |
There was a problem hiding this comment.
unrelated to this PR, but this idea of having a configuration for adding styles and attributes seems interesting also for interact-element and can help creating complex compositions (e.g. background-scroll)
| // Fallback for environments without Constructable Stylesheets (e.g. jsdom) | ||
| const style = doc.createElement('style'); | ||
| style.textContent = BASE_CSS; | ||
| (doc.head ?? doc.documentElement)?.appendChild(style); |
There was a problem hiding this comment.
this might not run. Maybe log a warning in that case?
|
|
||
| /** Return `true` when `segment` contains only whitespace characters. */ | ||
| export function isWhitespaceOnly(segment: string): boolean { | ||
| return segment.length > 0 && /^\s+$/.test(segment); |
There was a problem hiding this comment.
+ already captures that it is not empty
| return segment.length > 0 && /^\s+$/.test(segment); | |
| return /^\s+$/.test(segment); |
| const tokens: string[] = []; | ||
| let current = ''; | ||
|
|
||
| for (const { segment } of segments) { | ||
| if (!segment) continue; | ||
|
|
||
| if (isWhitespaceOnly(segment)) { | ||
| if (current) { | ||
| tokens.push(current + segment); | ||
| current = ''; | ||
| } else if (tokens.length > 0) { | ||
| tokens[tokens.length - 1] += segment; | ||
| } else { | ||
| current += segment; | ||
| } | ||
| continue; | ||
| } | ||
|
|
||
| current += segment; | ||
| } | ||
|
|
||
| if (current) tokens.push(current); | ||
|
|
||
| return tokens; | ||
| } |
There was a problem hiding this comment.
kind of weirdly written. It seems that using some while loop here should make it more readable
| * the result. | ||
| */ | ||
| export function getTextContent(element: HTMLElement): string { | ||
| return (element.textContent ?? '').trim(); |
There was a problem hiding this comment.
I have definitely seen this line in the other files (and not calling getTextContent), which kind of proves it is not enough to be an actual utility. If we want to keep it, then let's replace those lines with a proper call to this function.
| return NodeFilter.FILTER_REJECT; | ||
| } | ||
| } | ||
| return NodeFilter.FILTER_SKIP; |
There was a problem hiding this comment.
why skip? are we accepting only non-element nodes?
2 participants
Description