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

Customisable highlight style? #1191

Closed
gamar3is opened this issue Oct 1, 2024 · 13 comments · Fixed by #1202
Closed

Customisable highlight style? #1191

gamar3is opened this issue Oct 1, 2024 · 13 comments · Fixed by #1202

Comments

@gamar3is
Copy link

gamar3is commented Oct 1, 2024

Currently, the highlight style is somewhat hardcoded here.

Instead of setting a style attribute, would it be feasible to set a class instead, so that it can be changed through CSS?

Ideally this could be then redefined by the user through a function, but just having it as a class would enable me to change things with a userscript and try out other highlighting styles.

@gamar3is
Copy link
Author

gamar3is commented Oct 3, 2024

(btw i can try and make a PR for this, just asking to be sure that this feature fits into the project plans)

@felixroos
Copy link
Collaborator

I think we still need a style attribute to be able to set any color, which isn't possible by a class: https://strudel.cc/#cygiYmQqPDEgMj4gLSwgaGgoMTAsMTYpLC0gcmltIikuYmFuaygnQ2FzaW9SWjEnKQouY29sb3IoIltjeWFuIHwgbWFnZW50YSB8wqB5ZWxsb3ddKjgiKQouX3BpYW5vcm9sbCgp

@gamar3is
Copy link
Author

gamar3is commented Oct 4, 2024

Oh I see -- this is a tricky one. I'd really love to try out different styles for the highlights, e.g. with CSS transitions, so I'll keep thinking of a possible approach that doesn't break the color() feature.

Right now I'm thinking that adding a class (e.g. highlight-char) in addition to the style attribute would allow for this. The style attribute would likely override any CSS styles for the class, but maybe !important would address this.

I'll try to find some time during the weekend to set up a dev environment and test this out, but I'd definitely welcome any caveats you see with this approach.

Thank you for the quick reaction! I'm porting my work from Tidal to Strudel mostly because of the highlight feature, so I'm really eager to help make things even better.

@gamar3is
Copy link
Author

gamar3is commented Oct 4, 2024

I actually got this going -- adding the highlight-char class in packages/codemirror/highlight.mjs then editing the style in website/src/styles/index.css works. I'm delighted at how easy it is to hack on Strudel.

Now wondering if it makes sense to have a user-facing function to alter the highlight style. I'll read up on how Strudel does things like .color() and hopefully come up with something.

@matthewkaney
Copy link
Contributor

One thought: perhaps Strudel could have a css (or highlightCSS) param that could override the color param/default highlighting if set?

@gamar3is
Copy link
Author

One thought: perhaps Strudel could have a css (or highlightCSS) param that could override the color param/default highlighting if set?

I really like this idea, could be finessed later but would make a lot of things possible right now

@felixroos
Copy link
Collaborator

One thought: perhaps Strudel could have a css (or highlightCSS) param that could override the color param/default highlighting if set?

good idea! the rule could be: if css is set, color is ignored, and css is used for the style attribute. maybe also markstyle / markcss

@felixroos
Copy link
Collaborator

it should be noted that color is also used for the visuals (scope, pianoroll etc). the css wouldn't work for those, as it's canvas based. so I guess the behaviour for the viz would remain unchanged

@gamar3is
Copy link
Author

it should be noted that color is also used for the visuals (scope, pianoroll etc). the css wouldn't work for those, as it's canvas based. so I guess the behaviour for the viz would remain unchanged

That makes sense, and I can perfectly live with that limitation :-)

@felixroos
Copy link
Collaborator

image

@gamar3is
Copy link
Author

This is great and exciting! I suppose we could also call markcss autonomously (i mean, outside a chained sound statement) so it affects the whole sketch?

@felixroos
Copy link
Collaborator

This is great and exciting! I suppose we could also call markcss autonomously (i mean, outside a chained sound statement) so it affects the whole sketch?

you could use all for that, like all(x=>x.markcss('background:tomato'))

@felixroos
Copy link
Collaborator

now implemented in #1202 (soon to be deployed)

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

Successfully merging a pull request may close this issue.

3 participants