Enhance: refactor editor CSS

[eight04]

Since I didn't follow UI development, I can only guess what we tried to do from the code:

• We have a compact mode, which is basically just max-width: 850px.
• We have fixed-header, which is enabled when in compact mode & sections editor & scroll down a bit.
• We display #basic-info on top, and make collapsibles fixed.
• Collapsible stores its state to pref.
• In compact mode, we don't store expended state to pref? How about storing them as a different entry e.g. editor.options.compact.expanded
• In compact mode, we only display title in #lint.

While trying to accomplish these goals, I also re-arranged the code so selectors belong to the same section are grouped together.

[tophf]

I don't understand why is this needed?

[decembre]

As you rework the CSS editor, can you take in consideration these 2 old requests:
Make line counter continuous (not restart at 0 for each new section) #1096
[Feature Request] Extend editor with buttons for auto !important / go to the line / Comment or uncomment etc... #676

[eight04]

I don't understand why is this needed?

While fixing #1358 (comment), I found that our CSS code depends on many global states and it has to work with JS. Including cross browser bugs (I didn't see #1358 (comment) in FF), unused rules, conflicted rules, !important overwritten, etc.

Goals in this PR:

• The final goal is to remove JS dependency. Simplify the code. While fixing Add: style settings #1358 (comment). I have to look into edit.css, base.js, dom.js. In the future, we should only have to modify edit.css.
• If it has to depend on JS, make it a reusable component instead of a global state.
• Don't have to worry about breaking layout in the future. So we can freely modify the UI instead of always using modal.

Another thing that can be done in the future is to separate compact UI from normal UI, or build the UI from mobile first (which starts with less rules). Therefore, you don't have to worry about breaking compact layout when editing normal rules (or vice versa).

@decembre

Note that this PR doesn't modify the editor but the CSS code which styles the editor page.

[tophf]

I think you're overcomplicating and there's no need for this. It might be nice to do someday, but only as a part of a larger redesign. I'm against large refactors in HTML/CSS generally because they tend to break more than they solve.

[tophf]

Also, I see the bug #1358 (comment) in FF. I'm using Open editor in a new window and Use a simple window (no omnibox) options.

[eight04]

I think you're overcomplicating and there's no need for this. It might be nice to do someday, but only as a part of a larger redesign. I'm against large refactors in HTML/CSS generally because they tend to break more than they solve.

How about to treat this PR as part of a larger redesign? I'm against large refactors in HTML/CSS generally because they tend to break more than they solve and this is exactly what we are going to solve.

Also, I see the bug #1358 (comment) in FF. I'm using Open editor in a new window and Use a simple window (no omnibox) options.

My browser:

[tophf]

If you want to do a redesign then you should start by making mock-ups so we can agree upon it first.

Your screenshot shows the bug: the Code and Settings "tabs" aren't visible.

[eight04]

If you want to do a redesign then you should start by making mock-ups so we can agree upon it first.

Basically, the UI/features will be the same as I mentioned in the first post.

And the the final goal/benefit is described in #1365 (comment).

Your screenshot shows the bug: the Code and Settings "tabs" aren't visible.

Do you mean you'd like to make the tabbar fixed instead of scrolling with the page? It should be a feature instead of a bug IMO. Currently they scroll with the page in non-compact UI. We can open an issue for that later.

[tophf]

the UI/features will be the same as I mentioned in the first post.

So the UI won't change, but it should change per my suggestion to use a modal + button to show it.

Anyway, it's better to divide the PR into separate small changes and do them separately because there's no guarantee you don't add new bugs when doing a big refactor. If there's no big change in the UI then there's no need for a big refactor.

Do you mean you'd like to make the tabbar fixed instead of scrolling with the page? It should be a feature instead of a bug IMO. Currently they scroll with the page in non-compact UI. We can open an issue for that later.

Do you mean you don't see the "Code" and "Settings" labels are obscured and thus unusable?

[eight04]

Do you mean you don't see the "Code" and "Settings" labels are obscured and thus unusable?

Nope. In fact the tab bar also scrolls with the page in this PR. Here is the gif in case you haven't tested it:

[tophf]

Why "nope" if your screenshot already shows the bug?

This is exactly what I see both in FF and Chrome when I open the editor.

Edit: your screenshot shows a scrolled window, which is not what the bug is about.

[tophf]

BTW, that's the only thing that needs fixing at the moment, which is what I thought you would do, not refactoring.

[tophf]

Seems like to see the bug you need a long first section so that it's taller than the window.

[eight04]

I guess I don't really understand what the bug is (I thought I did). Anyway, if we want smaller PRs, I think we can:

1. Re-arrange edit.css without changing code so at least it is not a 1200 lines CSS file. We can probably split out reset.css, animation.css, edit-mobile.css.
2. Build a reusable sticky component (probably composed with a sticky-container and sticky-content). Drop fixedHeader.
3. Make an isolated header for compact layout (or at least isolated <details>). Simplify collapsible and detach it from xx-compact.
4. At this point it should be easy to modify CSS/layout without breaking stuff. The remaining problem is usercss mode though it has a simpler structure compared to others.

This gives me the same feeling as refactoring 2000 lines edit.js in the past 🤣

[tophf]

Let's not split css or add more js files until we start using a build system because each additional file increases page load time. The editor is already wasting ~100ms on that, which is grating on my nerves every time I look at the profiler. Thus, let's postpone extracting, reusing, isolating stuff until that time.

Since this PR is only about such refactoring I think it should be abandoned.

I can fix the bug myself if you still can't reproduce it after my last comment above.

[tophf]

Actually, the bug should go away if you simply restore the previous state of the editor before #1358 and implement the UI the way I suggested initially: in a modal shown after pressing a button.

[tophf]

Oh, I see you've added 5 external files that load synchronously (in the critical render path) in that PR, which is unacceptable. When you or I switch the UI to a modal, the files should be loaded on demand, after the button is clicked.

[tophf]

Closing because it adds more problems than it solves.