CSS Selectable Languages in Source Mode
Closed this issue · 28 comments
I know we've had a few CSS discussions. This is kind of separate to those. Currently in source mode (and reading mode), the language name in the header is implemented as text and no corresponding class is added to the element.
It would be great if for javascript for example, the div.codeblock-customizer-header-container would have the class language-javascript or some equivalent. This would finally allow for styling by language in live preview mode outside just the header!
It does have a class associated with it: codeblock-customizer-header-language-tag Or you mean you'd want a specific class for every language? So for C++ codeblock-customizer-header-language-cpp , and for CSharp codeblock-customizer-header-language-csharp? I could take the string what is defined in the code block, and set that as a class.
Yep, that would be what I was hoping for! That would be amazing.
EDIT: I mean adding the language specific class.
Yeah, no problem. I can do that.
This is done. Release 1.1.10 will include this.
Out of curiosity do you have an estimate for when you might release 1.1.10? It would probably be easier to submit my PR after the release.
You can submit it. That way I wouldn't had to test everything twice :) And maybe your changes help solve my problem. I only 1 or 2 problems with CSS. Unfortunately, I can't figure it out how to solve them, because all of my solution break something else.
Ok, I'll submit it. I've never actually made a PR before so apologies if it is pretty low quality. I'm happy to explain anything in more detail as well!
One last question, is there a hook that is triggered after the settings has been changed/closed/applied?
Not, that I know of. Why do you ask?
In the PR, I've currently extracted all the points where styles are applied and added them by classnames and css. Everytime a style.setProperty() was used, I changed it to document.body.style.setProperty() but this means that each of these is run many times for no reason in the program when it loops. They really only need to be set once - either on load or after a settings change. I'm not sure where to put them currently.
What were your css issues? I can have a go at solving those while I'm at it.
I don't think that's ok. Styles should be applied to that specific element where it is needed, not to the whole document. But I might be wrong. I can show the problematic stuff tomorrow, but now I have to go to sleep.
Hmm I'm having trouble working out what the accepted best practice is in Obsidian. I'll have a think tommorrow as well and let you know.
Sure good night.
So the problem I havin is the following. If you create in editing mode a very long line without a space, then it does not break as it should. It is pushed down a little, and thus it is not displayed next to the line number. But this only happens in editing mode. If the line contains at least one space then it is displayed correctly:
I think were something else too, but I can't remember. I'll have to check my notes after work.
I'll take a look.
With regards to the css scope question, can you point me towards any plugins that change colours in their settings?
Here is one I know: https://github.com/sissilab/obsidian-image-toolkit
Basically, I think there's a disparity between these types of plugins. That plugin uses each of the coloured options once at any given time (it is only applied to the selected elements). In your plugin, the same styling would be applied multiple times to many different elements so it makes much more sense to do it by class styling.
With the question of setting the css properties --highlight-colour on a global scale vs just at the element, my understanding is that you would ideally set it as few times as possible. If you had to set it for each line that is highlighted, you'd set it many times, but setting it globally and using a stylesheet to match classes for highlighted lines would only require you to set it to the body once. That's why I've gone with adding the styles to body. I'm not sure why you would want to limit the css scope to the highlighted lines. There is no danger of another plugin using a --codeblock-customizer-... property.
I've actually gone back to believing the colours should be implemented in the style settings plugin (which is how the plugins that I use mostly deal with customisation). Especially given the level of customisation this plugin offers. This would allow for the default themes and also let them be modified (as well as import and export to store themes easily). This wouldn't mean you have to install the style settings plugin at all. The default themes would be available and users could overwrite the css with stylesheets if they desire (though of course the easiest way would be through the style settings plugin).
Also in that style settings plugin, css variables are set for the selector body.css-settings-manager which suggests to me that setting css variables in the body scope isn't frowned upon.
What is this element doing? <img class="cm-widgetBuffer" aria-hidden="true" style="display: inline !important;">
In the obsidian discord I asked about how to do this best and the answer I got was:
'Lets say I have a highlight colour that the user can overwrite - --highlight-color, I can set a default value for it in a stylesheet but if the user can change it in settings, would I want to just use document.body.style.setProperty('--highlight-color',value)'
'I would think so. That is what I would do on value change and plugin load.'
So I believe the idea I originally followed in the PR is the correct one.
If you wish to avoid using the style settings plugin:
Furthermore, I'm not familiar with exactly how the pickr settings work, but on save of a pickr colour and on load of the plugin, the styles could be updated. Then no colours would actually ever need to be passed through the plugin.
Please let me know your thoughts before I continue on this track.
I don't want to limit the css scope to the highlighted lines. I just said, that as far as I know the styles should be applied to the elements. The reason I said this is simple. In every example they show you to set the styles to element.styles, where element is your element, and to the document.body. I have not seen a single plugin, where someone set something to document.body. If it does work your way as well, without bugs, and does not cause any performance issue, then I have no problem with that. But do not forget, it has to work with other themes as well, and that is where the problem begins sometimes :)
And yes, I still wish to avoid using style settings.
What is this element doing?
<img class="cm-widgetBuffer" aria-hidden="true" style="display: inline !important;">
I have no idea. That is not coming from my plugin. Obsidian inserts it automatically.
The difference here between those examples and this plugin is that you are setting the same styles multiple times. Currently you set a class AND set a style. The style is redundant since those classes can be used to style by themselves. If it were only one element, sure you could just set the element style but with 10 elements all with the same class requiring the same styling, rather than setting the element styles directly for each element, we can just set a css property on a scope encompassing all the elements and use a stylesheet based on the classes. This is definitely cleaner imo. As I said, I'm no expert, but I'd be surprised if you could find a plugin that changes element styles as much as yours does which applied the styles direct to elements.
I think I will continue with the PR as it started and you can decide whether to merge it or not when you review it. My PR should definitely work in the same way the plugin previously worked but I'll let you test that for yourself.
Also I believe this CSS fixes the issue you raised. It relates to this issue.
If there's anything else let me know!
.HyperMD-codeblock:has(> .cm-widgetBuffer) {
white-space: nowrap;
}
.HyperMD-codeblock:has(> .cm-widgetBuffer) > .cm-hmd-codeblock {
white-space: break-spaces;
}Yes, that is perfect! You're a genius! And I was banging my head into my desk for days over this. Oh man, you are definitely a life saver!
Now only one problem remains. Because of the new ln:{number} option:
If the starting linenumber is set to a bigger number, then the line number will not fit into the element, and not display it correctly. If you have an idea how to solve, I would be grateful! I am looking also in the meantime.
If you are ready with your changes do a PR, and I will review it, and also implement my current fixes and features and I can release it finally.
Thanks again for your great help and effort!
I've submitted the PR! It also includes a fix for the issue of the gutter colour not being maintained with the padding:
Thank you! I will review it and test it thoroughly just be to sure everything works fine. In worst case tomorrow I will release it.
You may want to actually leave off reviewing just a bit. I've actually fixed most of the issues I mentioned in the PR and will resubmit it.
@mayurankv are you on discord? Can you send me a message? Although I think I already messaged you :D