swup / astro

Astro integration for swup 🚀
94 stars 2 forks source link

Allow setting theme options #12

Closed daun closed 1 year ago

daun commented 1 year ago

Description

Allow passing in theme options as second argument:

{
  theme: ['overlay', { direction: 'to-right' }]
}

Checks

Additional information

Closes #11

hirasso commented 1 year ago

Why did you decide for the array form? I think I'd prefer a more explicit object like

{
  name: string,
  options: object
}
daun commented 1 year ago

@hirasso Mostly to keep it compact. I spent so much time in ESLint configs lately where the format of tuples for config also just felt natural:

"no-undef": ["error", { "typeof": true }]

Your suggestion is valid as well and is actually what I had implemented at some point. What bothered me about it was that customizing a single theme option would turn a single line into six lines if formatted normally. That's just a lot of visual noise, I felt like.

{
  // Before
  theme: 'overlay',

  // After
  theme: {
    name: 'overlay',
    options: {
      direction: 'to-right'
    }
  },

  // Versus
  theme: ['overlay', { direction: 'to-right' }]
}
hirasso commented 1 year ago

It's probably a matter of taste. I prefer clarity over density most of the time.

daun commented 1 year ago

@hirasso Definitely. If the thought of a tuple for this makes you shudder, feel free to push to this branch and I'll happily accept it. While I have a strong preference for it in this single case, I'm not opposed to the object version either.

hirasso commented 1 year ago

If you feel strongly about it, lets go with it.