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

[icon-themes] explore using attribute selectors to allow name patterns and simplify implementation #177650

Open
aeschli opened this issue Mar 20, 2023 · 4 comments
Assignees
Labels
feature-request Request for new features or functionality themes Color theme issues
Milestone

Comments

@aeschli
Copy link
Contributor

aeschli commented Mar 20, 2023

We currently use css classes to match file icons to file names.
Using attribute selectors could simplify this and allow to support name patterns

image

[basename^="hello-"][ext="ts"] {
    background-color: red;
}

[basename$="Bar"][ext="ts"] {
    background-color: yellow;
}
[basename="unique-Bar"][ext="ts"] {
    background-color: green;
}
<div basename="hello-world" ext="ts">hello-world.ts</div>
<div basename="hello-world" ext="js">hello-world.js</div>
<div basename="fooBar" ext="ts">fooBar.ts</div>
<div basename="barBar" ext="ts">barBar.ts</div>
<div basename="hello-Bar" ext="ts">hello-Bar.ts</div>
<div basename="unique-Bar" ext="ts">unique-Bar.ts</div>
@zm-cttae
Copy link

Very very nice. I like that this moves the heavy lifting for theme keys back to the file icon side. Minor notes:

  • The attributes contributed in iconLabel.ts need to use data- attributes for HTML5 semantic correctness.
    If we don't do this, we will get ⚠ warnings in the devtools console.
  • We can support surrounding wildcards (as in *selector*) using [attr*="selector"] in CSS service
  • If we want to implement the feature request in [icon-themes] Support for globs in file associations (Icon themes) #12493, we would need to add globbing in extensions

I believe I might have thought of this in #12493 but didn't devise it or sketch it out.
I guess I was just hyper-focused on getting the original use cases implemented in linear time 😄

@aeschli
Copy link
Contributor Author

aeschli commented Mar 20, 2023

Thanks @zm-cttae. I agree with all your points!
Yes, the idea is that the patterns can also be used for extensions
Still to be clarified is how we deal with multi-segment extensions: hello.test.ts. Right now we generate multiple class names test.ts-ext-file-icon and ts-ext-file-icon and icon themes can match again ts or test.ts

Maybe move the dot inside the extension and use the $ to match?

<div data-basename="hello" ext=".json">hello.ts</div>
<div data-basename="hello" ext=".test.json">hello.test.ts</div>

/* extension: "ts" */
[data-ext$=".ts"] { 
    background-color: red;
}
/* extension: "test.ts" */
[data-ext$=".test.ts"] {
    background-color: blue;
}

However, that would limit the patterns that can be used in extensions.

@zm-cttae
Copy link

zm-cttae commented Mar 23, 2023

We can use data attribute and ext class to cover full functionality I think? Sucks we can't do it in one..

@zm-cttae
Copy link

Would you like me to try refactor #174286 again, see if this could work?

@aeschli aeschli added this to the Backlog milestone May 3, 2023
@aeschli aeschli added themes Color theme issues feature-request Request for new features or functionality labels May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality themes Color theme issues
Projects
None yet
Development

No branches or pull requests

2 participants