search

lukePeavey / SplitType

Javascript utility that splits text into lines, words, characters for animation
https://lukepeavey.github.io/SplitType/
545 stars 39 forks source link

Add even BETTER typing for `type` and `split` options #62

Closed StevenJPx2 closed 10 months ago

StevenJPx2 commented 1 year ago

Right now, we have a naive combination for the three splitting options which makes it quite difficult to see what you want (repetitions, mainly). Adding a custom type to create true combinations to keep the suggestions nice, accurate and concise.

Before:

Screenshot 2023-08-13 at 9 44 45 PM

After:

Screenshot 2023-08-13 at 9 43 06 PM

I also experimented with tuple combinations, but TypeScript seems to flatten it down to just array element suggestions and stops suggesting combos after the third element. But it doesn't stop you from defining a fourth element, which is not what you want.

Another thing to note, I've changed TypesValue from being a union to a tuple, just to keep the typing as simple as possible. The first commit actually has a version where it converts the union to a tuple to do the combination, but it seemed unnecessary and too much for something simple, so I changed it instead.

Suggestion:

Maybe to make it stricter and allow JS interop, we can create an object that stores the values of the different combos.

SplitType.TypeOptions = {
  Lines: 'lines',
  Words: 'words',
  Chars: 'chars',
  LinesWords: 'lines,words',
  WordsChars: 'words,chars',
  LinesChars: 'lines,chars'
}

new SplitType({..., type: SplitType.TypeOptions.LinesChars})

This would limit the number of user errors if you limit them to only these options.

lukePeavey commented 1 year ago

Thanks this sounds great! I'll take a look at it as soon as I have a chance!

StevenJPx2 commented 1 year ago

Any updates?

lukePeavey commented 10 months ago

Any updates?

@StevenJPx2 Sorry! have been busy didn't have a chance to review this.

lukePeavey commented 10 months ago

@StevenJPx2 This is a nice improvement. The suggestions for the types option look much better!

Regarding your suggestion (explicitly defining all the possible values using an object), I'll leave it up to you. I think your current solution works well though.

I'm going to merge this, feel free to submit another PR if you want to make any additional changes.

Again, apologies for the slow response time

StevenJPx2 commented 10 months ago

No problem! Thanks for merging! I looked into the object style of doing it; thing is, if we want to enforce the user to only use the object, we probably need to brand the type, which makes it more complex. Haven't looked it more, but if I think it's feasible, I'll put in another pull request like you mentioned.