ether / etherpad-lite

Etherpad: A modern really-real-time collaborative document editor.
http://docs.etherpad.org/
Apache License 2.0
16.48k stars 2.84k forks source link

Facilitate skin customization/extending #3978

Closed brunob closed 4 years ago

brunob commented 4 years ago

Hi, at https://pad.infini.fr/ we use latest 1.8.3 with the new default skin customized with a plugin available here https://framagit.org/infini/ep_infini

First, thx for the work on this new fronted and for the new skin system :)

As you can see in our plugin, we are trying to achieve some small tweaks on "default" skin :

FTR, before upgrading to 1.8.3 we were using a customized theme bases on custom dir cf https://framagit.org/infini/themes-outils-libres/-/commit/6c71dc24b7619a07d6d27cb37b1c5da232148bd1 but i'd like to stick to the new skin as much as possible in order to facilitate our upgrades in the future :)

I'm not really satisfied of what i've done to achieve this, since i have to use eejsBlock_indexWrapper hook in order to insert the js and the css on home page, maybe the index page lack a new hook in order to add content in head of the page.

We also have to customize the index page in order to change go2Name() & go2Random() functions, but for now i haven't find a way to provide a template from a plugin.

I've read @muxator comment https://github.com/ether/etherpad-lite/issues/3441#issuecomment-422155860 saying that for now it's not possible to add another layer of customization on top of skins, and from what i've read here https://github.com/ether/etherpad-lite/issues/3471 "in case of errors, fall back to no-skin" it doesn't seem possible to "extend" colibris skin, since any file not provided by a personal skin will fall back to no-skin. Am i right ?

As i was discussing about this with @mrflos here https://framapiaf.org/@infini/104120325892102445 i think i'll have to end up wiht a plugin in order to insert our topmenu script on pads page + a skin for the index customization.

Some questions :

Anyway, i know that you are in a transitional state, since v2 will change things around this subject.

JohnMcLear commented 4 years ago

cc @seballot

seballot commented 4 years ago

Hi @brunob !

I guess what you can do is create a new skin folder, and import colibris css with

@import url("../colibris/pad.css");

?

For js... Cannot see an easy solution, but the js code in colibris skin is quite small so I guess it should be ok to copy it...

Re supporting customization, a solution could be to delete the "no-skin" and use only colibris as default. so we stop using the skin mechanism and we go back to the old way with the custom folder? But will need to go with the community to see if some people still want to use the old skin

From a development perspective, it's a bit a pain having to maintain both skins. You change something in the no-skin, it break something in colibris etc... And you always have to check both skins work nicely after any change in the template, or adding a new UI component etc...

brunob commented 4 years ago

@seballot thx for quick answer :)

In fact, i' totally agree with your comment here https://github.com/ether/etherpad-lite/issues/3498#issuecomment-433605239 and agree too on the fact that it's painfull and useless to maintain two "default" themes.

Default theme should be easy to extend, for css, js and templates. For now we have to create a plugin, a skin and customize templates with a patch, which make me think i could do all this directly with a patch :D But a patch is not a good way to ensure maintenance and versioning.

To be clear, i don't want to push the team in hurry and i understand that you are in a transition before 2.0 release, but maybe we could find a way to allow small customizations by listing the key points for that :

In fact, i miss all these facilities since "i live in a world called SPIP" where it's super easy to customize default templates/skins and also these customization with another theme/plugin :p

Edit : one point would be easier if no-skins were removed in 2.0 and replaced by colibris as it would allow people to extend only certain files of colibris in personal skins and let the fallback do its job for missing files.

brunob commented 4 years ago

For now i think i'll end up with an old good git diff and patch apply since i just want to apply this for example :

diff --git a/src/static/favicon.ico b/src/static/favicon.ico
index c33f2e614c63ec5f14e80dfce324416ffe4f47ed..04953ef40e67fc671d6748ec82952e9834cc76c6 100644
GIT binary patch
literal 1150
zcmd7Ru}eZx6vy!+Y7I(CNK;^gVDv9YuDOPT;U5q#F1oeoW+eRsZRsAZPAP(u=>H%%
zDA81k2-^4fxbS-D^KJQ)&-0#h?#0IiMSOgsQQ>(Zg@DMJh+Kje9jnim2(#0WWm6x{
z!)LvnzM*G&#w5?;9d91#UOi({V=>3~9zV=ySl|&R2^O~=WY}|!G|pjCWHCVB#gN$(
z^p2c)pfSeCh0b3Pgfnp(Hy)(ebA=e}sZe}kvd5J97oK6yJw*o}F51kx=%EaIk`y@?
zcg*z8BRIhk?5R_%u-rjsErhWxS(**J?$D%Pg7bH2{cqN55mn{i^v5x^@;h*YRhV<X
DJG5i=

literal 660
zcmV;F0&D$=P)<h;3K|Lk000e1NJLTq000mG000mO1^@s6AM^iV00004b3#c}2nYxW
zd<bNS00009a7bBm000ie000ie0hKEb8vp<R8FWQhbW?9;ba!ELWdL_~cP?peYja~^
zaAhuUa%Y?FJQ@H10uf0>K~y-6g_2EZ6Hyd}&wHKBOgc%&Xxb?f@rU{oTo&u1_(Otq
zA(0dzrbLlK7u|Fr6to~BE()&H6zZnCZY*>PqYD?JE<_|&s34V8V#STIqK!1uNn_%B
zF4_!ErZIQ*;GXl{bKiRe01U&>IOn_K@%RU9RZlc|f`BCfRKdtn=Eclpt!Eg<Zc1re
zEEdaB05C19uUx5)0YEyPZtp#l9HyK_J!RpBr!3r}gyoM%F5Vj+9qj;sqGg?5FO{zX
zfB*pZr{+Wg5&(eW1|RqPTT_K!W~m}pM>b~?fUuTOFmx6`cw%Z!0;8%8__x}3s$adb
z{A87zMMr}Z>`;}?dc6(s2!f>R`W;GXaUg#BQX-Kk)r+r>BvZPsPfpK1wesHzwQN97
zB=LZPTp@(4FwR-M(V2u3N)ce3e9t_eIV^0Z<5|w(RSyOCwTPbo`T=0=(+~Wye!sL=
zAb?3ILHD&NM4#Nm&apK87R!$p*By7j8AtBnI|z!{G(rB=XZ!4Ug|2b{KB3iR$Sr@t
zVm7xe%mElo^+W3jx<s$vEa2w&M03E{?YodA$tC)^TG-a?0$lEVgf^+-R&9MB0*={S
z7v@(qC%~_GeWtmA1L_)Nq1td71rSn9%tM1LiHZ#fAKZUr;r&MpJ&QIpJ3y3T?;b5&
ugQD+LcV?hJ(s*=T^&UIQE}T1=0f4`o4XOvNyn_q?0000<MNUMnLSTZHS{bDP

diff --git a/src/static/skins/colibris/index.css b/src/static/skins/colibris/index.css
index 0452d45f..25e0fc96 100644
--- a/src/static/skins/colibris/index.css
+++ b/src/static/skins/colibris/index.css
@@ -5,7 +5,7 @@ form {
 }

 body {
-  background: url(images/fond.jpg) center center no-repeat fixed #fff;
+  background: radial-gradient(circle,#283584,#1e2237);
   font-family: Roboto, Oxygen, Ubuntu, Cantarell, "Open Sans", "Helvetica Neue", sans-serif !important;
   font-size: 16px;
   line-height: 1.42857143;
@@ -47,8 +47,8 @@ body {
   text-shadow: none;
   font-size: 23px;
   line-height: 1.8;
-  color: #64d29b;
-  background: #586a69;
+  color: #fff;
+  background: #ec6825;
   border-radius: 3px;
   box-shadow: none;
   height: 53px;
@@ -61,8 +61,8 @@ button[type=submit] {
   border-top-right-radius: 3px;
   border-bottom-right-radius: 3px;
   left: 305px;
-  color: #64d29b;
-  background: #586a69;
+  color: #fff;
+  background: #ec6825;
   border: none;
   top: 0;
 }
@@ -70,9 +70,9 @@ button[type=submit] {
 #button:hover,
 button[type=submit]:hover {
   cursor: pointer;
-  background: #64d29b;
-  border: 2px solid #586a69;
-  color: #586a69;
+  background: #8f4018;
+  border: 2px solid #8f4018;
+  color: #fff;
 }

 #padname {
@@ -80,4 +80,15 @@ button[type=submit]:hover {
   max-width: 350px;
   padding: 0 12px;
   position: relative;
-}
\ No newline at end of file
+}
+
+#chapo {
+  color: #fff;
+}
+
+#chapo h1 {
+  margin-top: 0;
+  text-transform: uppercase;
+}
+
+.topmenu { position: fixed; top: 0; width: 100%; z-index: 10; }
\ No newline at end of file
diff --git a/src/templates/index.html b/src/templates/index.html
index 4962560b..5c4ce04a 100644
--- a/src/templates/index.html
+++ b/src/templates/index.html
@@ -37,6 +37,7 @@
         <link rel="localizations" type="application/l10n+json" href="locales.json">
         <script type="text/javascript" src="static/js/html10n.js"></script>
         <script type="text/javascript" src="static/js/l10n.js"></script>
+        <script type="text/javascript" src="https://www.infini.fr/spip.php?page=topmenu.js"></script>

         <style>
             html, body {
@@ -167,6 +168,11 @@
                     <input type="text" id="padname" maxlength="50" autofocus x-webkit-speech>
                     <button type="submit">OK</button>
                 </form>
+                <div id="chapo">
+                  <img src="https://www.infini.fr/plugins/spip-infini/img/logo-sendagi-blanc-inline.svg" alt="Infini" width="80" height="80">
+                  <h1>Infini</h1>
+                  <p>Hébergeur associatif de sites et services web</p>
+                </div>
             </div>
         <% e.end_block(); %>
         </div>
@@ -177,12 +183,14 @@
             function go2Name()
             {
                 var padname = document.getElementById("padname").value;
-                padname.length > 0 ? window.location = "p/" + encodeURIComponent(padname.trim()) : alert("Please enter a name")
+                var instance_id = Math.floor(Math.random() * 2) + 1; // 1 or 2
+                padname.length > 0 ? window.location = 'http://lite'+instance_id+'.infini.fr/p/' + encodeURIComponent(padname.trim()) : alert("Please enter a name")
             }

             function go2Random()
             {
-                window.location = "p/" + randomPadName();
+                var instance_id = Math.floor(Math.random() * 2) + 1; // 1 or 2
+                window.location = 'http://lite'+instance_id+'.infini.fr/p/' + randomPadName();
             }

             function randomPadName()
diff --git a/src/templates/pad.html b/src/templates/pad.html
index 45f9a7d3..4ac8fbe6 100644
--- a/src/templates/pad.html
+++ b/src/templates/pad.html
@@ -54,6 +54,7 @@
   <link rel="localizations" type="application/l10n+json" href="../locales.json" />
   <script type="text/javascript" src="../static/js/html10n.js"></script>
   <script type="text/javascript" src="../static/js/l10n.js"></script>
+  <script type="text/javascript" src="https://www.infini.fr/spip.php?page=topmenu.js"></script>

   <!-- head and body had been removed intentionally -->
JohnMcLear commented 4 years ago

This stuff could 100% just be a plugin surely?!

brunob commented 4 years ago

@JohnMcLear from all the docs, comments and issues i've read it doesn't, bu tmaybe you have some infos i haven't found ?

JohnMcLear commented 4 years ago

Surely just use eejs body hooks? Look at all the existing plugins that make similar modifications to the delivered html.

brunob commented 4 years ago

@JohnMcLear are you talking about https://etherpad.org/doc/v1.8.3/#index_eejsblock_name body hook which is only available for the pad.html page ?

JohnMcLear commented 4 years ago

I have plugins that are hooking into index.html...

https://www.npmjs.org/package/ep_list_pads

brunob commented 4 years ago

@JohnMcLear i've seen this one, but you don't insert any css or script, only html, and i'm already using the eejsBlock_indexWrapper hook in my plugin (cf my first comment) but i think it's more a hack than a clean method way to insert css and js on index.html :p

Thx anyway for answering :)

JohnMcLear commented 4 years ago

You can include script and CSS in the html.... You could just add blocks for Css and script too. That would be super easy

brunob commented 4 years ago

@JohnMcLear i'm already doing it :p https://framagit.org/infini/ep_infini/-/blob/master/infini.js

JohnMcLear commented 4 years ago

Send through a PR w/ the new hooks?

brunob commented 4 years ago

Sure, a first step would be allowing custom styles and scripts on index.html loke that :

diff --git a/src/templates/index.html b/src/templates/index.html
index 0e36f71c..aa7ad20a 100644
--- a/src/templates/index.html
+++ b/src/templates/index.html
@@ -157,6 +157,7 @@
             }
         </style>
         <link href="static/skins/<%=encodeURI(settings.skinName)%>/index.css" rel="stylesheet">
+        <% e.begin_block("customStyles"); %>

         <div id="wrapper">
         <% e.begin_block("indexWrapper"); %>
@@ -213,5 +214,6 @@
             if (typeof customStart == "function") customStart();
             // @license-end
         </script>
+        <% e.begin_block("customScripts"); %>
         <div style="display:none"><a href="/javascript" data-jslicense="1">JavaScript license information</a></div>
 </html>

If that looks good to you i'll provide a PR for that patch.

The second step is to externalize js code present in index.html in index.js in order to allow users to customize the go2Name(), go2Random() etc functions.

The third step, which need reflexion, would be to allow customization/extend of default skin without overriding all the files of this skin. For now, the default one is colibris but we can't set a personal index.css, for example, and let all the other files fallback to colibris ones (for now default files are in no-skin which is already overridden by colibris).

JohnMcLear commented 4 years ago

yep that's fine, you can just override those functions btw, I have seen other plugins do that. Afaik randompad or something does it :)

brunob commented 4 years ago

My patch seems wrong since it miss <% e.end_block i'll make a clean one in a PR to discuss about it.

The third point of my last comment is still open to discussion.

brunob commented 4 years ago

@JohnMcLear last but not least i'd like to provide a custom (local file, not an url) favicon from a plugin, i've read that comment https://github.com/ether/etherpad-lite/pull/1124#issuecomment-10162704 which mention something like this but i don't see how to achieve it. Do you have any clue for me about this ?

JohnMcLear commented 4 years ago

settings.json has the setting for favicon.

A plugin would just override the setting or so?

brunob commented 4 years ago

settings.json has the setting for favicon.

I've read about that, but it mention that it should be "a fully specified Url" so i'm not sure how to provide it from a local file in my plugin.

A plugin would just override the setting or so?

Not sure, i'm just curious about this part of the cited comment "This means, you could implement a plugin for customizing the favicon".

brunob commented 4 years ago

FTR @JohnMcLear i finally found the way to achieve the custom favicon from a plugin with the help of loadSettings hook.

https://framagit.org/infini/ep_infini/-/blob/new_hooks/infini.js#L59

It works well with #3881 patch, thx for your help on this, closing the issue :)