Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Aliasing with layered shadow support #250

Open
nclsndr opened this issue Sep 10, 2024 · 0 comments
Open

Aliasing with layered shadow support #250

nclsndr opened this issue Sep 10, 2024 · 0 comments

Comments

@nclsndr
Copy link

nclsndr commented Sep 10, 2024

I just noticed an issue with #246

The spec does not describe how aliasing should be supported along the introduction of the optional array-based value.

Let's take

{
  "simpleShadow": {
    "$type": "shadow",
    "$value": {
      "color": "#000000",
      "offsetX": "1px",
      "offsetY": "1px",
      "blur": "2px",
      "spread": "4px"
    }
  },
  "layeredShadow": {
    "$type": "shadow",
    "$value": [
      {
        "color": "#000000",
        "offsetX": "1px",
        "offsetY": "1px",
        "blur": "2px",
        "spread": "4px"
      },
      {
        "color": "#CCCCCC",
        "offsetX": "2px",
        "offsetY": "2px",
        "blur": "6px",
        "spread": "6px"
      }
    ]
  }
}

Following the current aliasing rules, I guess we allow:

{
  "referenceToSimpleShadow": {
    "$value": "{simpleShadow}"
  },
  "referenceToLayeredShadow": {
    "$value": "{layeredShadow}"
  }
}

But, do we intend to allow "partial aliasing" within the array value 👇 ?

  "layeredShadow": {
    "$type": "shadow",
    "$value": [
      "{simpleShadow}",
      {
        "color": "#CCCCCC",
        "offsetX": "2px",
        "offsetY": "2px",
        "blur": "6px",
        "spread": "6px"
      }
    ]
  }

I dunno to what extend this feature might be appealing for users, opinions here?

Yet, the current state of having the "shadow" type allowing both object and array based values makes the adoption of the feature almost impossible due to inconsistent spread/merge behaviours required to compute.
I described a solution addressing this in #239 - here and here

I guess that also relates to #241 - where it feels we'd need some simple rules we apply accros types with similar requirements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant