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

Switch WYSIWYG button is too close to the top. #170

Conversation

rotdrop
Copy link

@rotdrop rotdrop commented Feb 8, 2022

It partly covers the first line of the intro text.

It partly covers the first line of the intro text.
@solarkraft
Copy link
Contributor

solarkraft commented Feb 9, 2022

I don't exactly remember how it's implemented, but when I made my styling changes I made them with mostly the Argon/Krypton theme in mind. Whether the button is too high or not will always depend on the theme around it, so longer term the switching button should be integrated into the editor buttons instead, IMO.

@rotdrop
Copy link
Author

rotdrop commented Feb 9, 2022

Ok, I'm just using the standard theme. I agree that the way the button is implemented is a little bit of a kludge as it is more-or-less impossible to position it in a theme-agnostic way. OTOH, integrating it in the editor buttons would imply that also the standard editor needs to have a switch-editor button, or at least an extension point. So maybe we close this pull request and rather add an issue "refactor switch-editor button" or something like this.

@solarkraft
Copy link
Contributor

So maybe we close this pull request and rather add an issue "refactor switch-editor button" or something like this.

I guess it makes sense to do both. I find it reasonable that the button should look the best in the default theme.

Such an issue has already existed with #92. Maybe it could be reopened (or another one created, sure).

FYI, in my development repository I already have an issue for further reworking of button. It would go along well with fablab-luenen#9.

These issues are in my own repository because I meant to work on them myself, don't want to impose my vision on anyone, and, tbh, because this repository seems a bit unmaintained.

Would you be interested in working on these? I haven't been able to allocate many development resources to this project lately.
If you think it makes sense to open fablab-luenen#10 in this repo, by all means, do so. I'm not too familiar with the ideal GitHub issue flow yet, so I'm not sure what makes the most sense in this case.

@solarkraft
Copy link
Contributor

solarkraft commented Feb 13, 2022

I noticed that in narrower configurations the button with your settings also seems to cover the text:
Screen Shot 2022-02-13 at 15 34 04

There are really not many ways around this without a proper rework, I think. The only workaround I can think of would be moving the button even higher and to the left (top: -1rem; right: 3rem;):

Screen Shot 2022-02-13 at 15 36 30

... but that too is not very elegant, IMO.

A proper workaround to get the button into the layout flow would require changing the location at which it is injected. Currently it is at the end of the editor ...

Screen Shot 2022-02-13 at 15 42 02

... a better location might be behind the intro paragraph ...

Screen Shot 2022-02-13 at 15 44 26

... or inside it.

Screen Shot 2022-02-13 at 15 45 46

This would still be worth doing despite "properly" reworking the button, since it will be needed when the visual editor isn't active.

My favorite would be something like this:
Screen Shot 2022-02-13 at 15 51 05

since the visual editor should be very easy to enter, but a bit harder to leave (it should be the preferred mode of editing, after all).

A textual explanation would probably also do some good for confused users (really, nobody should be able to get stuck in markup mode):

Screen Shot 2022-02-13 at 15 58 58

@annda
Copy link
Contributor

annda commented Aug 9, 2023

Fixed.

@annda annda closed this Aug 9, 2023
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

Successfully merging this pull request may close these issues.

3 participants