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.

[Mottie]

I opted to hide the entire block if you choose to find styles inline. I also fixed all the other issues I mentioned above.

[narcolepticinsomniac]

I opted to hide the entire block

I doubt it's used much, but it does serve a purpose. More importantly, it's consistent and looks nicer remaining where it belongs when you scroll back up. I'd much rather change it back the way it was.

Still a bunch of cosmetic issues with icons and their surrounding elements in terms of sizes, padding, and positioning. I can hook this stuff up today or sometime soon. Not like we're rushing with this PR.

[narcolepticinsomniac]

I cleaned up those cosmetic issues, and a couple bugs from changes I forgot to incorporate when combining PRs.

@Mottie Regarding the wiki icon, I didn't like the look of two circle icons right next to each other. I made a square info icon that's fairly decent. I preferred the "W", but using a "Wikipedia" icon for "wiki" seems to bug you, so I tried to come up with something else.

[narcolepticinsomniac]

I think I fixed usercss applies-to theme detection decently. It ignores the default CM theme since it's already overridden with light gray and more detailed styling.

@eight04 I'm guessing you wrote it to begin with. It hasn't been updated to include add/subtract icon styling. I added the bg color to select option because the select styling for dark themes is done with semi-transparency, so most were white on white. Also, our select elements have hover/focus state styling now, so it'd be nice if these did too, but NBD I guess.

@Mottie either of you guys are welcome to tweak the condition if there's a better way to do it. Seems to work fine though.

[narcolepticinsomniac]

@Mottie regarding the thumbs-up icon for "OK", I looked at a bunch and that was actually the least shitty. It's a Github icon. You're welcome to try to find a better icon if you want.

[eight04]

@narcolepticinsomniac Removing the style completely doesn't really work. The widget would try to extract colors from the editor gutter so it can make sure that the color of the text/border/background will match the editor theme. After deleting the style entirely, you can see that the background of the applies-to-widget is now brighter than the editor gutter.

---

TBH, I don't like the new theme after trying it. Everything is grey and lack of contrast.

Icons in the popup are confusing, you don't know what they do until hovering on them and looking into the tooltip. You can't tell if they are clickable since they are all grey.

In the manager, since the entire page is already "shadowed" by grey, the box-shadow between two areas becomes obscure and lack of effect.

Here are some screenshots:

Popup

Before

After

Style manager

Before

After

Editor

Before

After

Dialog

Before

After

Options page

Before

After

---

I didn't follow previous PRs. Why do we want to make the entire interface grey?

[narcolepticinsomniac]

Removing the style completely doesn't really work.

Of course it does. actualStyle is empty if the CM default theme is enabled.

After deleting the style entirely, you can see that the background of the applies-to-widget is now brighter than the editor gutter.

All styling for the default is styled via .CodeMirror.cm-s-default (to override CM's built-in stylesheet) in our edit.css/codemirror-default.css files, including the gutters, applies-to, and everything inside of it. We can make it match if you want.

Icons in the popup are confusing

Layouts are optional, simply switch back to the classic UI if you prefer it. One of the most consisitent complaints is that the UI is ugly, and when it comes to the popup, I agree. The majority of users interact with the popup more than anything else, and all the actions being unpredictable length strings inside links and buttons is a messy waste of space.

you don't know what they do until hovering on them and looking into the tooltip

Same with pretty much any icon-heavy layout. There's a handful of new, main icons. They all seem very obvious to me, but even if they weren't, there are tooltips. If you prefer the look, how long would it take you to remember what a handful of icons do?

You can't tell if they are clickable since they are all grey.

They're all clickable, except for the "write-style-for", which is just an indicator for the adjacent action, but it does have a useful, and more descriptive tooltip, so while not exactly an action, it does serve a purpose other than just being a text replacement.

Everything is grey and lack of contrast.

Everything gray and lower contrast was the goal. Last time I asked the peanut gallery for opinions, 90% preferred the default theme to be light gray over bright white. We can ask again, but I suspect the results will be similar.

...the box-shadow

Really?

[DanaMW]

Just a note Narco I make my clickable icons sort of flash on mouseover that or i give them a slick twist or lift maybe that is something you might like and maybe i could help you separate the menu from the whole ui so it is its own option? i know the new one will look better than the default but i would like to separate simply the icons from ui in favor of the buttons. Just a thought or two. Thanks

[narcolepticinsomniac]

i would like to separate simply the icons from ui in favor of the buttons

There are two prefs for the pop UI in options. Buttons or icons, and style name click action, toggle or edit. If you select buttons and toggle, the layout is exactly as it's always been, or you can to mix and match the two, same as I told you the other day. No doubt some will prefer the classic, that's why it's still there.

...clickable icons sort of flash on mouseover

They change to black, the cursor changes to a pointer, and the tooltip tells you the action. That's as obvious as any icon layout. Should we add a keyframe to make them dance around?

i could help you separate the menu

I would enjoy putting you to work. Unfortunately, as usual, I have no idea what you're even talking about here. =)