-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add heading level and alignment to inspector #1383
Conversation
This is really really interesting. I can't quite yet tell whether I like it, but I think I love it. But it feels like we need more opinions on this one. CC: @melchoyce @mtias @folletto @afercia Here's a screenshot: |
(Also, I still have on my todo to make better icons for the H levels, but I'll do that separately) |
I have a similar positive gut reaction! |
Given the positive reactions, let's get this in master and test it. |
1ee7224
to
420f995
Compare
components/toolbar/style.scss
Outdated
@@ -36,6 +36,11 @@ | |||
height: $icon-button-size - 16px; | |||
} | |||
} | |||
|
|||
.editor-sidebar & { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to include these styles in editor-sidebar
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @jasmussen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 👍
I'm a bit conflicted with moving the alignment, because now some blocks will have alignment in quick access, and some will have it in the inspector. |
@mtias Yeah, I wonder if we should move it to the inspector everywhere, as an advanced setting. |
Matías makes a good point. Should we show instead .. and then the rest in the inspector? Instead of what it is now, |
I don't mind that, or show alignment in the inspector for all blocks. An argument against the latter is that centre alignment is used a lot. Something in between we should explore: centre alignment toggle for all blocks (one button), and more alignment options in the inspector. N.b.: alignment has nothing to do with LTR and RTL. I saw some confusion a few months ago. |
I think I'd rather see the switcher in the inspector, to be honest. The other part of the conversation is that we've said the inspector is for advanced settings, and the design optimizes for that—hidden on mobile, etc. Specially if it become a modal, it would be awkward to have alignment there because you cannot see what you change with a modal obscuring it. |
I'd also rather see it in the inspector. Regarding the mobile argument, the same could be said about all other advanced controls as well, e.g. drop cap. Not sure how alignment would be different. |
Part of the "basic & advanced split" conversation is also the implicit tradeoff that we are okay with it not being as accessible on mobile. That said, Google Docs does something interesting on mobile: We could presumably do something similar with our inspector, but it could only be for block level actions, as inline actions would invoke the soft keyboard. |
Just an idea. See #751.
This PR removes levels 1, 4 and 5 from the inline toolbar, and adds levels 1-6 to the inspector. Also adds alignment.
@joen Feel free to style any way you want.