Skip to content

Conversation

@nsaunders
Copy link
Owner

Description

This change drops the keyframesName function and exposes the KeyframesName constructor in its place.

Design considerations

First, a little background: The keyframesName function was intended as a smart constructor "placeholder" of sorts. I got the idea that it might be important when I noticed that a <keyframes-name> value is a <custom-ident> meaning that certain values aren't allowed, such as certain keywords. Although the function's signature keyframesName :: String -> KeyframesName didn't convey the notion of failure (e.g. through a Maybe KeyframesName return type), my high-level plan was to eventually add some logic to make any invalid String input into a valid KeyframesName value by changing the input in some way.

Now I am thinking this would likely involve a complicated implementation, likely an imperfect one, and probably doesn't offer enough benefit to justify that cost.

For comparison, I also had a look at Halogen, whose ClassName type (moved to web-html in 2020) is analogous to KeyframesName. Here's what I found:

  • No smart constructor; the ClassName constructor is exposed instead. See Web.HTML.Common.
  • There used to be a className constructor function, but it was removed many years ago without explanation.
  • The aforementioned className function never included any validation or other logic to ensure that the ClassName return value would be a valid class name (e.g. free of whitespace). Perhaps this is why it was pruned.

Thus, in removing keyframesName I am following suit with the leading PureScript UI framework, whose core maintainers' judgment I trust, even if they didn't document their rationale in this case.

Moreover, I am considering (#29) adopting the ClassName and AttrName types (from web-html, previously halogen). Since their constructors are exposed directly, I think the same pattern should be followed here for consistency's sake.

Future plans

Revisit this issue if changes are made in Web.HTML.Common.

References

Code change checklist

  • Any new or updated functionality includes corresponding unit test coverage.
  • I have verified code formatting, run the unit tests, and checked for any changes in the examples.
  • I have added an entry to the Unreleased section of the CHANGELOG.

@nsaunders nsaunders merged commit 5bc2d2d into master May 12, 2023
@nsaunders nsaunders added enhancement New feature or request breaking Non-backward-compatible API change labels May 13, 2023
@nsaunders nsaunders deleted the feature/drop-fn-keyframes branch July 27, 2023 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Non-backward-compatible API change enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants