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

compiler: class should take priority over glob #1766

Open
alixander opened this issue Dec 8, 2023 · 7 comments
Open

compiler: class should take priority over glob #1766

alixander opened this issue Dec 8, 2023 · 7 comments

Comments

@alixander
Copy link
Collaborator

**.style.fill: green
classes: {
  x: {
    style.fill: red
  }
}

a.class: x
b

a should be red and b should be green. Currently they are both green.

@gavin-ts
Copy link
Contributor

gavin-ts commented Dec 8, 2023

I think whichever comes later in the d2 file should take priority, in case you want to use a glob to set classes or override a value in a class

In your example a.class: x overwrites the glob style so it still works as you expect.

However you may want to do something like the following where you overwrite a class with a glob:

classes: {
  x: {
    style.fill: red
    style.stroke: yellow
  }
}

a1.class: x
a2.class: x
b.class: x
a*.style.fill: green

@alixander
Copy link
Collaborator Author

implementation note:

I think the only way is to reimplement classes into the IR.

Since globs are at the IR level, the IR produced will show a and b having style green as if they were set without globs. And of course, styles set without globs should take precedence over classes.

@alixander
Copy link
Collaborator Author

I think whichever comes later in the d2 file should take priority, in case you want to use a glob to set classes or override a value in a class

@gavin-ts setting classes is orthogonal to this issue.

As for order, globs apply to current objects and lazily to future objects. You can stick *.style.fill: green anywhere and it'll apply to the whole scope. But it won't override a.style.fill: red, whether it comes before or after. So for classes, it should have same behavior.

@gavin-ts
Copy link
Contributor

gavin-ts commented Dec 8, 2023

As for order, globs apply to current objects and lazily to future objects. You can stick *.style.fill: green anywhere and it'll apply to the whole scope.

right

But it won't override a.style.fill: red, whether it comes before or after. So for classes, it should have same behavior.

shouldn't it override a.style.fill: red if it comes after that ? but a.style.fill: red overrides the glob if the glob is first?

This is how labels work

Screenshot 2023-12-07 at 9 58 10 PM

@gavin-ts
Copy link
Contributor

gavin-ts commented Dec 8, 2023

I think whichever comes later in the d2 file should take priority, in case you want to use a glob to set classes or override a value in a class

@gavin-ts setting classes is orthogonal to this issue.

agree not class specific but in general shouldn't setting anything later in the file overwrite what is earlier? whether it is classes, globs, or labels such as the example above?

@alixander
Copy link
Collaborator Author

Mechanisms have different priorities over each other, and ordering matters within those same priorities.

If I have a shape with style.fill set, I don't think setting a class on it that also has style.fill should override.

To me, the rationale is specificity.

Globs target most.
Classes target second-most.
Individually setting properties target only individual shapes.

So more specific rules override less specific.

@gavin-ts
Copy link
Contributor

gavin-ts commented Dec 8, 2023

To me, the rationale is specificity.

Globs target most. Classes target second-most. Individually setting properties target only individual shapes.

So more specific rules override less specific.

I can see the specificity rationale. However in the example reposted below the glob is used to target something more specific than the class. With those rules the glob would be ineffective and you may need a separate class for a's just to set the fill (and you wouldn't be able to override that class with a glob a*.class: x-with-green-fill if they already have a class set)
What are the rules for assigning a class with a glob if an existing class is present? (it seems it would also be ineffective with the specificity rules)

As a tangent, is there a good way to add an additional class to an object (rather than overwriting all classes)?

However you may want to do something like the following where you overwrite a class with a glob:

classes: {
  x: {
    style.fill: red
    style.stroke: yellow
  }
}

a1.class: x
a2.class: x
b.class: x
a*.style.fill: green

If I have a shape with style.fill set, I don't think setting a class on it that also has style.fill should override.

I can see that but wouldn't it be possible to just move the class definition before the shape-specific styles (or vice versa)? Having the property later values override earlier ones seems like a nice and useful property allowing globs to be much more capable e.g. used as above. This could also be useful to use classes to override a set of values (maybe you want to reset styles to a certain class in a scenario/step?)

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

No branches or pull requests

2 participants