Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI element visible on read-only pads #13

Open
thmo opened this issue May 1, 2020 · 21 comments
Open

UI element visible on read-only pads #13

thmo opened this issue May 1, 2020 · 21 comments

Comments

@thmo
Copy link

thmo commented May 1, 2020

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
Copy link
Member

JohnMcLear commented May 1, 2020

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
Copy link
Author

thmo commented May 1, 2020

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
Copy link
Member

They should, do they not?

@seballot
Copy link

seballot commented May 2, 2020

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

@JohnMcLear
Copy link
Member

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

@Oremountainflorian
Copy link

They should, do they not?

At least ep_font_color and ep_align don't hide.

@thmo
Copy link
Author

thmo commented May 19, 2020

See also ether/ep_align#16.

@JohnMcLear
Copy link
Member

Ack will deal with it if I can

@JohnMcLear
Copy link
Member

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

@seballot
Copy link

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

@JohnMcLear
Copy link
Member

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

@JohnMcLear
Copy link
Member

@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
Copy link
Author

thmo commented May 19, 2020

@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
Copy link
Member

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

@JohnMcLear
Copy link
Member

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
Copy link

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
Copy link
Member

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
Copy link
Member

@seballot you on with fixing this up?

@seballot
Copy link

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

@JohnMcLear
Copy link
Member

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
Copy link

thanks ! fixed here ether/etherpad-lite#4017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants