[SGrid] Support a way to set X-axis gap and Y-axis gap differently

[NozomuIkuta]

Currently, gap prop set same value to both X-axis and Y-axis gap. Sometimes, there are design where they are different.

The new interface must be backward compatible otherwise the existing SGrid styles will be affected.

<SGrid gap="24" /> <!-- should be treated as `gap: 24px` -->

<SGrid gap="16 24" /> <!-- should be treated as `gap: 16px 24px` -->

[kiaking]

Good point. Like it. But maybe using tuple would be better...?

<SGrid gap="16" />
<SGrid :gap="[16, 24]" />

[NozomuIkuta]

But maybe using tuple would be better...?

<SGrid gap="16" />
<SGrid :gap="[16, 24]" />

Yes, using tuple is also fine. I suggested space-separated string so that 2 cases can be handled by same interface and same decoding logic:

<SGrid gap="16" /> <!-- acceptable as before -->
<SGrid gap="16 24" /> <!-- newly acceptable -->

defineProps<{ gap: `${number}` | `${number} ${number}` }>()

const gaps = computed(() => {
  const [ row = null, column = null ] = props.gap?.split(' ') ?? []
  return { row, column }
})

With tuple, the interface and decoding logic would be a bit more complex:

<SGrid gap="16" /> <!-- acceptable as before -->
<SGrid :gap="[16]" /> <!-- type error or acceptable? -->
<SGrid :gap="[16, 24]" /> <!-- newly acceptable -->

defineProps<{ gap: `${number}` | /* acceptable? */ [number] | [number, number] }>()

const gaps = computed(() => {
  if (Array.isArray(props.gap)) {
    const [ row = null, column = null ] = props.gap
    return { row, column }
  } else {
    return { row: props.gap, column: null }
  }
})

Another reason is that I thought using dynamic binding (i.e. :gap="value") seemed to be redundant because gap would not be changed in most cases once it's set.

[kiaking]

I think handling space will be harder to find error. I don't think props.gap?.split(' ') is enough. What if we confused and did this? gap="16, 24". or gap="16,24". or gap="16 24" (two space). With tuple, this won't happen 👀

I think destructuring tuple is easier than parsing string 😅

[cuebit]

A common practice is to use a standard format that's consistent and widely used in components already. For example, if most props that represent units are handled by number, or number[], it should be kept that way. Throwing varying degrees of formats in some places adds to that confusion, even when well documented. After all, when an error occurs, it's likely someone will look at the source before resorting to reading the docs.

[NozomuIkuta]

I think handling space will be harder to find error. I don't think props.gap?.split(' ') is enough. What if we confused and did this? gap="16, 24". or gap="16,24". or gap="16 24" (two space). With tuple, this won't happen 👀

I think destructuring tuple is easier than parsing string 😅

It seems that we are not on the same page.

The type of gap prop in the example above is defined as template literal type instead of simple string type.

defineProps<{ gap: `${number}` | `${number} ${number}` }>()

This means the gap is a string of number or space-separated 2 numbers. So there is no need to consider cases where other characters such as , are included, because it throws an type error.

This is why props.gap?.split(' ') is enough.

const gaps = computed(() => {
  const [ row = null, column = null ] = props.gap?.split(' ') ?? []
  return { row, column }
})

One trade-off of this approach is, the type of gap prop is compiled as null, which means "any type". But, as long as we don't set a value to gap prop which is unknown until we run app, it will not cause any problem.

// defineProps<{ gap?: `${number}` | `${number} ${number}` | number }>() becomes

props: {
    gap: { type: null, required: fals }
  },
}

// defineProps<{ gap: string | number | number[] }>() becomes

props: {
    gap: { type: [String, Number, Array], required: false }
  },
}

---

But, I don't have such strong opinion on this topic. I just wanted to suggest what I think is good and reasonable enough based on my knowledge. So adding array of numbers to gap prop is totally good just because it works (as I said in the first comment)!

Whatever approach we take, I believe this conversation was good to have because the team came to know template literal types are useful (and powerful) in some cases where we want strings to be formatted in some way. We may come to need this another time in future.

[kiaking]

Ah indeed template literals seems cool idea 😳 Though, I think @cuebit has a good point. This pattern both string with space and tuple never show up on another part of Sefirot, and I don't think this particular prop deserves to introduce new rules.

I think having multiple props is more natural for us 👍

<SDesc gap-x="16" gap-y="12">

[NozomuIkuta]

I think having multiple props is more natural for us 👍

I agree. 👍

I think it is fine to introduce the 2 props as a breaking change, without keeping single gap prop and treating it as gap-x internally as needed, because we will be able to fix the old usage in one PR when upgrading Sefirot. I will leave it to you, though.

---

Template literals would be also helpful when we want keys of objects to be formatted. For example, we can introduce it for i18n keys with TSDoc annotations so that the codebase can be self-documented and it will be easier for everyone to follow current and future conventions. (How much we should do it is another point to discuss, though).

Here is a demo.

[kiaking]

I think it is fine to introduce the 2 props as a breaking change, without keeping single gap prop and treating it as gap-x internally as needed, because we will be able to fix the old usage in one PR when upgrading Sefirot. I will leave it to you, though.

Ah, no, I was thinking to keep gap as well. That's we used to do too. Like :value and :model-value for Inputs. Having shorthand props are useful 👀

[brc-dd]

Just a heads up - gap: <y-gap> <x-gap> (not x, y) - it will be better to keep it gap-row and gap-col instead 😅