ether / ep_font_color

Font Color Plugin for Etherpad
Apache License 2.0
2 stars 15 forks source link

UI element visible on read-only pads #13

Open thmo opened 4 years ago

thmo commented 4 years ago

The drop-down to change the color of selected text is visible also for read-only pads. Using it temporarily changes the color of the selected text in the current browser window. The original pad is not changed however ([WARN] message - Dropped message, COLLABROOM for readonly pad), and the change vanishes after reloading the page.

Sometimes however the read-only pad looses the connection to the server when using the drop down.

JohnMcLear commented 4 years ago

Yea I'm yet to discover a good practice for which plugins should/shouldn't show on read only pages. @orblivion and @seballot did you have a thought on this? Something like a class that hides it on read only pages perhaps? I know we have the settings.json blob but I don't wanna be messing with that...

How about class='buttonicon-editor-only' ?

thmo commented 4 years ago

I would expect that ep_font_color, ep_font_family and ep_align all work the same way in that regards, similar to ep_font_size.

JohnMcLear commented 4 years ago

They should, do they not?

seballot commented 4 years ago

why not just hiding the whole left-toolbar in real-only mode? (I never used this mode, not sure what is the goal)

JohnMcLear commented 4 years ago

There has been cases where read only plugins get placed on left side of the toolbar so e can't do that.

Oremountainflorian commented 4 years ago

They should, do they not?

At least ep_font_color and ep_align don't hide.

thmo commented 4 years ago

See also JohnMcLear/ep_align#16.

JohnMcLear commented 4 years ago

Ack will deal with it if I can

JohnMcLear commented 4 years ago

@seballot before I hack something do you have a preference on how I do this?

seballot commented 4 years ago

your idea of having a class that will be hidden seems good to me

JohnMcLear commented 4 years ago

@seballot how is ep_headings2 not included in read only pad view?

JohnMcLear commented 4 years ago

@seballot I'm just thinking and you might be right, we might be suitable just to say "Anything that's put in the toolbar left can just be hidden" - I reviewed a load of plugins and it seems fine to do so.. the exception being ep_comments which one might want to allow comments on a page...

thmo commented 4 years ago

@seballot how is ep_headings2 not included in read only pad view?

From peeking and poking around in the inspector, I would think the acl-write class has this purpose?

JohnMcLear commented 4 years ago

@thmo thanks for doing my homework for me :D Awesome, so I think I will just include this in those plugins.

JohnMcLear commented 4 years ago

Wait, @seballot the bug is coming from font_color.css which isn't comin gfrom my plugin! It's coming from the colibris skin :P

It's due to

#font-color {
  display: list-item !important;
}

This is why I reallllly don't like plugin code in core ;\

seballot commented 4 years ago

ah ah sorry, it was because I dont really like this double action things : first click the "A" button, the select appear, then click the select, the list of color appears So out of laziness I made the select always visible and hide the "A" button, instead of changing the plugin itself

I'm a bit tired of managing all those plugins. I think the plugin system is very nice to add custom functionalities or heavy functionalities (like ep_webrtc), but for all basic actions like changing text size, text color, align text etc... should IHMO definitely be part of etherdpad core. it would be much more easy to have a clean and nice toolbar (it will still be possible to activate/deactivate some feature of the toolbar), and also it will be less time consuming

JohnMcLear commented 4 years ago

Yeah agreed. Let's fix these last ones up. Fwiw with do webrtc we are setting best practices so most of this functionality can be provided by plugin helpers.

JohnMcLear commented 4 years ago

@seballot you on with fixing this up?

seballot commented 4 years ago

how do I activate the read only mode? so I can test it?

JohnMcLear commented 4 years ago

Click the embed button (in toolbar), click the read only switch, get the read only URL, visit it :)

https://video.etherpad.com/p/r.b9c133871647a4d8b5d0c1ce5599fbf9

Is an example

seballot commented 4 years ago

thanks ! fixed here https://github.com/ether/etherpad-lite/pull/4017