Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.Sign up
RichText: collapse toolbar #14233
This PR solves two problems:
The arrow down is the same as the one in the block switcher and alignment switcher, so it doesn't feel too much.
Rich text dropdown:
This is the same kind of menu as the alignment dropdown:
How has this been tested?
Types of changes
requested review from
Mar 5, 2019
Wow, this is really nice! GIF:
It's not the magical wonderous responsiveness that we've been dreaming of, where you scale the block toolbar by width and stuff happens magically inside. But it's a solid step in the right direction. Your argument about making the cutoff at the "strikethrough" is solid to me as well — and honestly I feel this paves the way for us to add text color and inline text highlight tools to core as well. And finally it doesn't block even better responsiveness tools to land in the future.
I pushed a small fix to balance the margins so the dropdown is centered when hovered:
Can we add a tooltip as well? Accessibility wise it seems good to me, but it might be nice to get a sanity check here as well.
The block alignment and the alignment toolbars both use the
IIRC just wrapping any button in a
Thanks for the work on this thus far! I love that we're thinking about how we can accommodate smaller screens, and I like the ideas y'all are exploring. I have a one tiny bit of drive-by feedback with regards to mobile.
Beyond the personal/visual preference of the dropdown icon feeling too small (which I agree with @youknowriad on), it appears that the button doesn't provide adequate tap area. Unless my eyes are playing tricks on me, it looks like the current buttons are 36x36, so for now I'd at least match that. Then in the future I would consider going with ≥44pt (via iOS HIG) or better yet ≥48dp (via Google Material).
Obviously that concern isn't limited to this specific button, and it's another separate project, but maybe something to consider/keep in mind moving forward. :)
jasmussen left a comment
Yep, this is solid. Couldn't break it on any breakpoint, or with fullscreen or top toolbar or anything. It solves a problem and takes steps in the right direction. It's also a good place for the Inline Image button.
It's entirely possible additional enhancements can be made in the future, but this is a solid PR.
referenced this pull request
Mar 13, 2019
I'm not sure this is a good thing to do. It closely resembles the "Toolbar Toggle" button in the classic editor which was one of the most "questionable" functionality there. My (unofficial) stats were that about 90% of the people would open the second toolbar row there and leave it open ...forever (guessing the other 10% didn't know what that button does, and were afraid to test it) :)
Shouldn't the toolbar be roughly the same width as the block? Both visually and functionality wise that seems best. It would "fit" quite nicely visually, and at the same time won't obscure some of the (more useful) buttons.
Then, what about the alignment buttons. They are pretty much useless in "all text" blocks/areas like the paragraph block. I see in the screenshots that they are in another drop-down/popup. If that is implemented would we still need to hide the arguably more useful toolbar buttons added by plugins?