codemirror / dev

Development repository for the CodeMirror editor project
https://codemirror.net/
Other
5.91k stars 376 forks source link

Merge View: `collapseUnchanged` doesn't allow customizing "⦚ N unchanged lines ⦚" bars text #1467

Open VasylMarchuk opened 5 days ago

VasylMarchuk commented 5 days ago

Describe the issue

The squiggly lines of the "⦚ N unchanged lines ⦚" text, hardcoded into collapseUnchanged bars for expanding hidden lines are cool, but we unfortunately couldn't fit them into our theme design, and the text isn't customizable :( It took us significant efforts to customize-hack the text with ninja :before and :after CSS elements, but the implementation is clumsy and unreliable.

Details

Знімок екрана 2024-11-09 о 17 06 57

Amount of ninja CSS we had to use for customizing the bar & text

Знімок екрана 2024-11-09 о 17 10 23

Suggestion

It would be great if we could provide a callback, or something, in collapseUnchanged options, that would be passed the number of lines, and return the text for those bars.

Reproduction link

We have a complete functioning demo of CodeMirror, wrapped into an Ember.js component over at https://app.codecrafters.io/demo/code-mirror, configurable with most of the standard extensions & their options in realtime, please feel free to test over there. It's ok if people discover the link here on GitHub, but please don't link to it, yet :)

Знімок екрана 2024-11-09 о 16 36 38

All of the documents used as examples in the Demo's document drop-down can be found here. The Ember.js component itself, which wraps CodeMirror, passes it all enabled extensions, and handles updates is here, just in case.

Our current CodeMirror & Merge versions

npm ls --depth=0 | grep codemirror
├── @codemirror/language-data@6.5.1
├── @codemirror/merge@6.7.2
├── @uiw/codemirror-theme-github@4.23.6
├── codemirror@6.0.1
marijnh commented 5 days ago

The text is configurable via phrases, but indeed, the squigglies aren't. Would moving those into CSS before/after elements work for you?

VasylMarchuk commented 5 days ago

Not really @marijnh, it wouldn't, for several reasons:

VasylMarchuk commented 5 days ago

But also, there's a bigger problem with the "Expand xxx changed lines" bar, and I was about to create a separate issue just for this, but I guess I'll just put it here:

Please note how in, for example, VSCode and Github, the "Expand xxx changed lines" bar either extends into the gutter, or has a dedicated gutter element:

Знімок екрана 2024-11-10 о 14 06 02 Знімок екрана 2024-11-10 о 14 07 26

The problem

The way CodeMirror renders this bar — it is siblings with other lines in the document, it doesn't overlay the gutter, nor does it have a dedicated gutter element. This leads to a problem of the "gap":

Знімок екрана 2024-11-10 о 14 12 44 Знімок екрана 2024-11-10 о 14 11 41

Details

We have spent several weeks trying to solve this issue in various ways, coming up with CSS hacks, border color tweaks etc, but border is too noisy, gap is too noticeable. In addition, gutters div has varying width, depending on enabled gutters — and this voids any possible easy CSS solutions:

Знімок екрана 2024-11-10 о 14 12 11

Our temporary solution

Unfortunately all we could come up with, for now, is this implementation, our :before element has a crazy negative left margin and extends way beyond all possible gutters, but we couldn't solve the big padding before icon & text issue:

Знімок екрана 2024-11-10 о 14 21 38
VasylMarchuk commented 5 days ago

So TLDR:

UPD: I really should have stayed on point with customizing the text / removing the squiggles, sorry! Would you like me to create a separate issue for expand unchanged bar + gutter difficulties? But at least you know the background story now 😀

marijnh commented 4 days ago

we're already using the :before element to add an icon expand-diff-top|middle|bottom.svg

Yes, and that'd override the default style, which is what you want here, right?

in general using :before and :after triples the needed styles, as now you have to specify background & border + hover state + mousedown state, and everything else for all three — the element, the :before, and the :after — they aren't inherited

:before/:after elements definitely do inherit the styles of their parent element.

Does gutterWidgetClass help with your gutter styling? If not, please do open a separate issue.

VasylMarchuk commented 4 days ago

:before/:after elements definitely do inherit the styles of their parent element

They do for some, but not for all, for example background and border aren't inherited, which are the ones we actually care about most:

.decorate-me {
  margin: 30px 50px;
  padding: 50px 30px;
  background: red;
  border: 3px solid green;
}

.decorate-me:before {
  content: "Pseudo text";
  outline: 1px dashed navy;
  margin-left: -70px;
}

image

In addition, if we set the same background color for both the element & pseudo-element: we can't use rgba, because the pseudo gets transparency over transparency -> a different color.

Also: when trying to set an svg icon as a background for the :before element, I couldn't find a way to specify the svg color: color doesn't apply, and using fill: currentColor in the svg itself doesn't work either. We had to use separate svg files for icons in hover state, mousedown state, and then again for the Dark skin: so 18 icons in total.

VasylMarchuk commented 4 days ago

Please let me spend a couple hours to experiment with the squiggles being in before/after pseudo elements and if that helps address our concerns, I will come back to you once I know.

UPD: Still experimenting, quite tricky to simulate on the fly, plus had another urgent task.

marijnh commented 4 days ago

They do for some, but not for all, for example background and border aren't inherited, which are the ones we actually care about most:

Those never are—CSS inheritance generally only applies to properties where inheritance makes sense.