CSS refactoring (flexbox, animations, skin variants)

[seballot]

As we discuss earlier with @muxator and @JohnMcLear here is a pull request to improve the css code base

Main idea was to simplify the code, i.e. stop using absolute positioning lead by javascript, stop using table for positioning elements, and use flex box instead to make the design more responsive and auto adjustable

Note re the pull request

• I made one big commit (the last one), sorry for that but it was hard to split it while developing (when changing the UI you constantly do back and forward). I hope that is okay. I made the commit message more detailed than usual
• This pull request breaks the colibris skin. Another pull request will come to adjust colibris skin
• Some plugins (especially the ones who used new element positioned, like table_of_content) will probably be broken. I will have a look to different plugins, @JohnMcLear your help would be very appreciated :)

[seballot]

Actually, please do not review this PR right now, I will still work a bit on it tomorrow, and also run and fix the tests !

[JohnMcLear]

Note that I also worked on the tests so do pull my branch with fixed tests.

[seballot]

I fixed ep_table_of_contents and ep_set_tilte_on_pad, thanks @JohnMcLear for having merged the PR !

Do you see others plugins that might break the UI? I mean plugins which insert content inside the main layout?

[JohnMcLear]

ep_webrtc ?

[JohnMcLear]

ep_comments / ep_comments2?

[seballot]

Thanks ! Is there an easy way have ep_webrtc working for real in local?
Or do you know an online etherpad instance using this plugin?

[JohnMcLear]

@seballot just install ep_webrtc and install some certs from letsencrypt on a domain. You need to run https, if not maybe it will work in exceptions?

[seballot]

I just have this empty container for webrtc (I put it on the right)

[JohnMcLear]

anything on console?

[seballot]

WebRTC ERROR: DOMException

[JohnMcLear]

will check.

[JohnMcLear]

Hrm, try remove other plugins and try against latest develop (lots of change lately on develop).

It works for me without anything in settings.json, I just see

Sorry, your browser does not support WebRTC.

To participate in this audio/video chat you have to user a browser with WebRTC support like Chrome, Firefox or Opera. Find out more

I could add a protocol check and if it's http mention why it's not working... I just expect everything to be on https nowadays but I guess if you are running locally...

[JohnMcLear]

I'd remote any ep_webrtc settings too, might be a bug in that?

[seballot]

Thanks ! but if I just have this message it will not help very much :) I wanted to see the others components of the plugin (like where the video appears etc...) to ensure everything works. You do not know an online instance?

Unfortunately settings https on local host in a bit a pain

[JohnMcLear]

https://video.etherpad.com

[seballot]

Amazing ! thanks :) https://video.etherpad.com/p/test

[muxator]

Can I have a brief recap on the status of this and #3726?

[seballot]

Hi muxator !

For this one : etherpad core is now using flexboxes to easily positioning element on the layout (no more absolute positioning with javascript like $(editbox).css('top', $editbar.height())

It simplify a lot the code (both javascript and css) but this change is not compatible with plugins introducing others components in the layout with absolute positioning. I'm am currently fixing those plugins (table_of_content, set_tilte_on_pad, webrtc). But it introduce a compatibility problem between etherpad core version and plugin version, see discussion here about webrtc : ether/ep_webrtc#28 (comment). Your help would be welcome to find a good solution

What remain to be done in this PR is fixing the colibris skin

re #3726, I will wait this refactor to be done so I could rebase the changes on top of it

[seballot]

@muxator @JohnMcLear I gave more thoughts re the problem of compatibility between core code and plugins

Seems to be that the only way to improve the codebase without compatibility problem is to :

• Refactor etherpad removing javascript absolute positioning -> done in this PR
• leave plugins as they are (meaning using absolute positioning), so they will still be compatible with any version of etherpad -> TODO revert some of the pull request I made on ep_table_of_content, title_on_pad, webrtc...
• Add some strong css rules to etherdpad 1.9, so that the javascript positioning made by other plugins on the main containers will have no effect (so old plugins will not break the css in etherpad 1.9) -> TODO in this PR
• Add some css in etherpad core for positioning plugins in the new way (flexbox). It will not be a lot of code, few lines per plugins, seems like only 4 plugins introduce new containers in the layout, other plugins just add buttons or popups below toolbar -> TODO in this PR

WDYT?

[JohnMcLear]

I don't mind dragging people up to 1.9 to implement the new CSS.

The point is we need to make it clear WHY something is broken if it is, currently it fails silently. That's not good enough.. console.warn("upgrade yo shit"); for example...

[seballot]

Hi ! I fixed all specs (except the slider one), and also the ability to copy the pad text when we are disconnected

re the the pad_modal test, actually is was not the gritter popup, but the normal popup like settings or so, and the problem was just how we detect that those popup are open or not

But still made some improvement for gritters

[JohnMcLear]

Wow great job!! I absolutely can't wait to show people this work

[muxator]

It's hard to express the amount of work @seballot has done on this PR. It's gargantuan.

I'm focusing on the js changes:

• farbtastic and gritter are two old vendorized libraries that this PR edited. I've added a note on top of those files (Edited by Sebastian Castro [...] on...) to track the fact that they are no longer the original version. This may come in help when in need of reconstructing the history of those files;
• in the commits of this PR I've stripped the changes (mainly aestetics) that where part of Sebastian's version. In this way the diff consists only of his modifications. I liked his version better, but this is a big PR, and it's important to be able to asses what has changed;
• I have a potential security doubt in a change in src/static/js/chat.js. I'll address that later.

Overall, looking at this PR's diffstat for the js files, it's interesting to observe that the amount of javascript code has indeed decreased, despite the introduction of new functionalities (skin variants, animations).

[JohnMcLear]

We should be looking to contribute to gritter and farbtastic with some changes instead of going off piste.

[JohnMcLear]

I'm gonna write some front end test code that finds every input and tries to inject some xss

[muxator]

we should be looking to contribute to gritter and farbtastic with some changes instead of going off piste

In general, I totally agree. In this case, these libraries were already vendorized: hopefully who decided to vendorize them had already done a cost/benefit analysis. We are paying that cost now.

[muxator]

The skin variants builder at http://localhost:9001/p/test#skinvariantsbuilder is super cool.

We have to mention it somewhere prominent!

[muxator]

Merged, many thanks for the good work! 🚀

This is an awesome advancement in the quality of the frontend code.

[muxator]

Can I ask something personal?

I am not particularly fan of flat buttons with no borders. Would it be terribly difficult to have an options for having light borders on the formatting buttons?

It would be ok even if having a border just around the button groups, just not completely flat.

Is it possible?

[seballot]

Hi ! thanks for having reviewed the code so quickly, and for the nice and instructives comments. I did not thought of XSS :)

re farbastic and gritter, not sure we should submit those small changes, they are quite custom for etherpad, and not developed with the idea of being reused

re skin variant, yes I find it also quite fun. I think I will add it in etherpad.org when I will change the screenshots

re flat design, yes we could add a variant to enable or not flat design. The non flat design (=skeuomorphism design) could add small shadow on the editor, some border for the buttons etc.. Will think about it ! If you have some good examples of website with nice non flat design, you can give them ! Maybe github is one of those examples?

[JohnMcLear]

RE farbtastic and gritter, the point is more that we will need to update those packages periodically and when we do we don't want to lose your customizations..

[JohnMcLear]

@seballot Can we document a few things on new issues. -- Each line item warrants a new issue imho.

1. Gritter changes required
2. Farbtastic changes required
3. Basic step-by-step modernization of plugin guide (IE changes required to CSS classes etc). Maybe doing an example plugin update?
4. How to access the new theme creator thing :D

[JohnMcLear]

1. I reached out to gritter maintainer asking him if we can adopt that project.

[muxator]

I reached out to gritter maintainer asking him if we can adopt that project

I am not sure if we really need this. Have a look at the git history of gritter.js and your past self will probably agree. 😄

It was copied in the repo in 2013 and modified a few times, last one in 2015. Does not feel like something we cared to maintain. And by now it's a very custom thing.

[JohnMcLear]

Well at a bare minimum we should mention that it's a modified version of an original project and that the technical debt is ours to maintain and we are unable to commit back.