One UI PR to rule them all

[narcolepticinsomniac]

Probably best to load fresh. Whoever tests first can tell me how broken it is. I'm sure I forgot a file somewhere, but my local copy works. =)

[narcolepticinsomniac]

Optional popup icon UI with optional style name click actions, and light gray default theme combined. I wanted to tweak the theme to incorporate tophf's suggestions, mainly using semi-transparency wherever possible for userstyle compatibility, and leaving a lot of the text/icon contrast alone, particularly in the manager.

I also decided to convert all our colors to effing variables. If I hadn't underestimated what a PITA that was gonna be, I never would've started, but I think it's pretty complete. This involved a ton of text replace and manual editing without double checking every single edit individually, so it'll need to be tested thoroughly. I wouldn't be surprised if I missed some altogether either. To make it even more userstyle friendly, I incorporated some common html selectors for solid dialog BGs which are often overlooked.

Variables leave the door open for implementing a color slider for the UI, which would make the effort worthwhile. Otherwise, they'll make userstyles easier, and adding new elements simpler for us in the future by just adding the common selector.

I also monkey-patched the highlighting bug. In the default "token under cursor" regular highlighting interfered with "find" highlighting. In "selection only" regular highlighting didn't work at all. I always had it disabled, so I never noticed, but neither the JS nor the accompanying CSS made any sense or worked together.

AFAICT, highlighting has never worked correctly since match-highlighter-helper.js was introduced almost a year ago. I can only guess that it's gone unreported because power users who edit and use "find" in the default "token under cursor", also use themes which override the default highlight styling. Same with "selection only", which is completely unstyled in the default, and probably not all that popular anyway.

One of the main goals of match-highlighter-helper.js is to optimize highlighting animations, so transitioned highlights are only applied if there's multiple matches. If there's multiple matches, .cm-matchhighlight-approved is supposed to be added to .CodeMirror, if not, it's removed.

The condition is match-highlighter-helper.js ~line 130:

const num = ++this.highlightHelper.occurrences;
if (num === 1) {
  stream.lineOracle.doc.cm.display.wrapper.classList.remove(HL_APPROVED);
} else if (num === 2) {
  stream.lineOracle.doc.cm.display.wrapper.classList.add(HL_APPROVED);
}

Pretty much works in "token under cursor", but the default highlighting CSS still interfered with "find" highlighting. The condition doesn't work at all for "selection only", and the CSS makes even less sense since it relies on the .cm-matchhighlight-approved selector for styling, but isn't included in the keyframe that selector is meant to optimize.

edit.css line 356: body[data-match-highlight="selection"] .cm-matchhighlight-approved .cm-matchhighlight

body[data-match-highlight="token"] .cm-matchhighlight-approved .cm-matchhighlight,
body[data-match-highlight="token"] .CodeMirror-selection-highlight-scrollbar {
  animation: fadein-match-highlighter 1s cubic-bezier(.97,.01,.42,.98);
  animation-fill-mode: both;
}
body[data-match-highlight="selection"] .cm-matchhighlight-approved .cm-matchhighlight,
body[data-match-highlight="selection"] .CodeMirror-selection-highlight-scrollbar {
  background-color: rgba(1, 151, 193, 0.1);
}

Anyway, I had no luck fixing the condition, so the monkey-patch was to fix "selection only" by leaving it excluded from the animation and fixing the CSS to not rely on the .cm-matchhighlight-approved animation optimization selector. Then I fixed regular highlighting interfering with "find" highlighting by toggling a selector on body when search is opened so we can style all editors off of that. Bonus, now when you close "find", search highlighting goes away, which is pretty standard.

Everything works now, but we should really fix the condition so "selection only" can also have transitions which are optimized. They're a nice touch.

[narcolepticinsomniac]

Testing it briefly, I don't think I forgot any files, but I'm guessing something in edit.js isn't compatible with something eight04 changed recently in the master. Some of the UI is bugged. Hopefully he can shed some light, but I don't feel like screwing with it right now.

@Mottie @eight04 Both of you are welcome to fix/improve whatever in this PR.

[narcolepticinsomniac]

Took a little break, but leaving it broken was bugging me. Think I resolved all the conflicts.

[narcolepticinsomniac]

Didn't even occur to me that I'd need to check the html diffs, but I did. Everything appears to be working now.

[eight04]

Just a quick skim:

1. I've rewritten the entire backend and the section editor in Add style exclusions. Closes #113 #360 (and probably the popup this day). I think we should start working on the UI after that.
2. The fix and the discussion for Smart highlighting is conflicted with search results #513 should be pulled out to another PR.
3. The atomic CSS approach doesn't work well in our situation. Since we allow users to style our extension pages, we should keep using semantic class names.

[narcolepticinsomniac]

I've rewritten the entire backend and the section editor in #360 (and probably the popup this day).

What does one have to do with the other? If you're saying you're planning on screwing up something major in this PR, then that's a bad idea because I'm not doing it a third time. If you're not gonna cause major conflicts, then what does it matter? There's no big rush.

I think we should start working on the UI after that.

I started working on this a couple months ago.

The fix and the discussion for #513 should be pulled out to another PR.

Again, who cares? They needed to be styled correctly, and I was restyling the UI.

The atomic CSS approach doesn't work well in our situation.

I strongly disagree when it comes to the .main-bg selector for overlay dialogs. These are often missed by authors, and not everybody uses variables. The rest of the "atomic CSS" can go. There's not a lot, and they don't have much significance.

[Mottie]

Do we really need 26 levels of gray? I think a bunch of these could be combined.

Also I don't think "normal" humans can discern the differences in alpha channel between .05. .06, .07 and .1. Can we combine these as well? I think every 10% is okay, but not anything less than 5%.

[narcolepticinsomniac]

.06 makes a difference when stacked on top of another to achieve a combination shade. There's also a very discernible difference between .05 and .07 on elements like zebra-striping.

[narcolepticinsomniac]

I actually did combine quite a few shades of gray. You could probably eliminate at least a few more, but when you're converting the massive volume of values, you can't really go hunting for every element to check if a slightly different shade is detrimental.

[eight04]

The fix and the discussion for #513 should be pulled out to another PR.

Again, who cares? They needed to be styled correctly, and I was restyling the UI.

If the PR is small, we can review and merge it faster. Also, it is a good idea to focus on one thing in one PR.

I've rewritten the entire backend and the section editor in #360 (and probably the popup this day).

What does one have to do with the other? If you're saying you're planning on screwing up something major in this PR, then that's a bad idea because I'm not doing it a third time. If you're not gonna cause major conflicts, then what does it matter? There's no big rush.

The preference system is refactored and codemirror-editing-hook no longer exists:
https://github.com/openstyles/stylus/pull/517/files#diff-30bce154ae698318f0875aa5902cef86R302

[narcolepticinsomniac]

Removing [data-match-highlight] doesn't really matter, so we can lose that too. No CSS depends on it, I just thought it was cleaner and might come in handy in the future.

If it's just a matter of putting this PR off for a bit because it's less essential, that's NBD. As far as there being some urgency to merge a highlight fix, it's been broken for almost a year and no one has even reported it. Since that's the case, "reviewing and merging it faster" doesn't really feel that important.

Like I said, it's probably because power users use themes which override the bugs in the default. You're welcome to separate and merge the monkey-patch as it is. It's just the .find-open toggle on body in terms of JS, and the couple lines of updated CSS which are clearly referenced. Ideally, we could also fix the condition and tweak the CSS so "selection only" also has optimized animations as well, but I somehow doubt that mode is very popular, so also NBD.