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

G2: UI surrounding the editor canvas #19348

Merged
merged 11 commits into from
Feb 20, 2020
Merged

G2: UI surrounding the editor canvas #19348

merged 11 commits into from
Feb 20, 2020

Conversation

youknowriad
Copy link
Contributor

@youknowriad youknowriad commented Dec 27, 2019

bulids on top of #19344 with some additional items that can be harder to land without changing the whole canvas:

  • Dropdowns with darker borders
  • Updated border-less panels
  • Darker borders for sidebar, header and footer.

@jasmussen
Copy link
Contributor

I rebased this off G2 (also recently rebased), and started initial work.

But I'm starting slowly with this, because we are getting into some somewhat deep surgery of the editor — a reduction in colors (3 shades of gray instead of 10), a reduction, and other simplification work. For that reason in my first commit I focused only on the popovers in ba4fa80 — yet that still touched a lot of files, so feel free to glance over them to get a flavor for some of the change work we're looking at.

And it's not even done, it's only this far:

Screenshot 2020-01-22 at 12 56 37

This needs padding and spacing alignments, the icons and keyboard shortcuts need to be on the right, and we need a special heuristic for the popover component: most of them should have the light gray border and shadow, but all of the popover menus inside the editing canvas should have the dark border and no shadow.

Because there are always opinions on this, I don't want to move too fast.

@jasmussen jasmussen mentioned this pull request Jan 23, 2020
9 tasks
@jasmussen jasmussen changed the title Try/g3 G2: UI surrounding the editor canvas Jan 23, 2020
@jasmussen
Copy link
Contributor

I took the liberty of renaming this PR. Can we rename the branch also? If it's tricky, no trouble.

@youknowriad
Copy link
Contributor Author

We could rename the branch but I think it would required creating a separate PR. So maybe just keep it as is.

}

.components-menu-group__label {
text-transform: uppercase;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't all-caps considered worse for accessibility?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't aware they were.

I personally think that for a menu group label, which is decidedly seconary (light gray, smaller font size), it's okay to use it as a visual element, it helps the label also act as a separtor. However I'm open to suggestions, any thoughts @pablohoneyhoney?

@jasmussen
Copy link
Contributor

Made slight progress on simplifying the toolbar, which includes introducing a basic grid, rearranging the top left buttons, and moving the document outline to the footer (fixes #4287):

Screenshot 2020-01-24 at 12 04 41

Please do not consider the above representative of anything other than work in progress.

While the simplificaiton of the toolbar (grid, buttons), work is very far from done, and honestly this reduced border-style makes even clearer that we are in need of icons that fit together better. As the UI is reduced, it increasingly becomes clear that we are using a mix of icons and styles here. To no-ones fault, but a good reason for a fresh singular approach.

transition: color 0.2s ease;

// Spot color button.
// @todo: This button should ideally be 32px.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inserter is used in different places and not just the top toolbar. I think the "blue button" shouldn't be solved using CSS but using the isPrimary prop in JS and only for the top toolbar inserter.

@ZebulanStanphill
Copy link
Member

One thing that has always confused me is that the Save Draft and Preview buttons use different styles. I would think that they should look the same.

Also, should the Document Outline button use a visible label? I would think that there is enough space for it. I'm also not convinced it belongs in the footer. Perhaps it should be an extension similar to what Yoast and other plugins have, with the ability to pin it to the right-side of the top toolbar.

Finally, I wonder if the Block Navigation menu belongs in the footer instead of the Document Outline. It has a similar function to the breadcrumb links. Or maybe they both belong there.

@jasmussen
Copy link
Contributor

One thing that has always confused me is that the Save Draft and Preview buttons use different styles. I would think that they should look the same.

I still hope that we can entirely deprecate Save Draft as a vestige of the past. However it's increasingly clear that this is still a ways out in the future. In the mean time, yes, it does make sense to unify on a single button style. Though it is worth noting that there might land a different interface for the Preview button, when the editor gets responsive features and registers an editor style.

Also, should the Document Outline button use a visible label? I would think that there is enough space for it. I'm also not convinced it belongs in the footer. Perhaps it should be an extension similar to what Yoast and other plugins have, with the ability to pin it to the right-side of the top toolbar.

IMO, it should be a word count visible label, yes. Like so:

Screenshot 2020-01-27 at 10 10 38

Then you click it, and the document outline interface shows up. The icon isn't particularly helpful there, but a label showing real time information combined with a tooltip feels like a good balance to me.

Finally, I wonder if the Block Navigation menu belongs in the footer instead of the Document Outline. It has a similar function to the breadcrumb links. Or maybe they both belong there.

Possibly. While I think the Selection mode and breadcrumb interfaces both have made strides in selecting child blocks, I don't think we'll know how efficient those two are until we actually ship the G2 efforts: specifically the changes to what has focus and what is selected, in my experience of working on the branches so far, make it much clearer what goes on.

That's a roundabout way of saying that until we feel confident enough that those two interfaces work for selecting parent blocks, perhaps we should keep the block navigation menu where it is? And if they prove themselves, your points do make sense that the button could serve as an extension/expansion of the breadcrumbs.

@ZebulanStanphill
Copy link
Member

Also, should the Document Outline button use a visible label? I would think that there is enough space for it. I'm also not convinced it belongs in the footer. Perhaps it should be an extension similar to what Yoast and other plugins have, with the ability to pin it to the right-side of the top toolbar.

IMO, it should be a word count visible label, yes. Like so:

Screenshot 2020-01-27 at 10 10 38

Then you click it, and the document outline interface shows up. The icon isn't particularly helpful there, but a label showing real time information combined with a tooltip feels like a good balance to me.

I agree that the icon isn't that helpful in the footer, but I don't think the word count makes for a good button to open up the Document Outline, since it is not a description of what the button does. We would have to use hidden labels on screen readers, and for everyone else it wouldn't be clear that word count is a button in the first place.

@jasmussen
Copy link
Contributor

I don't think the word count makes for a good button to open up the Document Outline, since it is not a description of what the button does.

I actually quite disagree with this — the button shows a word count and shows word-count related information when clicked.

I do agree it could use additional sanity checking, so I'll add a label and invite that.

@jasmussen
Copy link
Contributor

I rebased. Here's a little taste test to whet the appetites for a merge into g2:

g3

I noticed that when the Top Toolbar option is enabled, the movers still do the smart unhiding. We should probably just toggle that off up there. @ItsJonQ can you help with that? Can commit straight to the try/g2 branch now :)

@ItsJonQ
Copy link

ItsJonQ commented Feb 19, 2020

@jasmussen Taking a look when I can :D

@ItsJonQ
Copy link

ItsJonQ commented Feb 19, 2020

@jasmussen Haiii! The Movers seem to be behaving on my end :o. I've been resizing desktop <-> mobile, as well as loading the page at various viewport sizes.

In doing so, I discovered this:

Screen Shot 2020-02-19 at 9 48 00 AM

Not sure if this already exists. It's when the viewport is exactly 782px

@jasmussen
Copy link
Contributor

Documenting to fix either in this branch or in g2 or in a followup: the block toolbar when seen in one of the responsive views (from the new dropdown) becomes a broken mix of what it is in the top toolbar and a by-the-block version.

@jasmussen
Copy link
Contributor

Q can you clarify what the issue is in that screenshot, it's hard to tell?

Also, what I mean about the top toolbar is when it's permanently on:

Screenshot 2020-02-19 at 15 53 25

In that case the movers keep their appear behavior:

Screenshot 2020-02-19 at 15 53 32

It seems to work fine on the mobile breakpoints where the toolbar is forced up there.

@ZebulanStanphill
Copy link
Member

@youknowriad

It doesn't work that well for the sibling inserter but I believe the sibling inserter doesn't work at all as a "primary" button (blue) because it's not a primary action anyway.

I agree; the sibling inserter shouldn't be considered a primary button.

@jasmussen
Copy link
Contributor

Good thoughts.

I think many of these thoughts are based on the half-step, even if it's still a big step, that this branch is. The sidebar needs a LOT of love, and a white header takes a step towards that. But I'm fine to explore that in a followup, so it's now gray again, even if that looks out of place with the reduced color set — (most of the rest of the sidebar also looks out of place :D ):

Screenshot 2020-02-20 at 09 48 14

I also made the block library button in the toolbar into an isPrimary button, which allowed me to remove a bunch of CSS. I kept it 32x32 as it's part of aligning those buttons to a 12px grid. I'm happy to also revisit this in a followup and either change that to 36, or explore the 32 in other places.

By making the inserter a primary, ironically, it also made the sibling inserter primary, which meant I had to undo some of those visuals to make it appear not primary:

Screenshot 2020-02-20 at 09 48 20

I don't have strong feelings for the current state of the sibling inserter, I profoundly thing that inserters inside the canvas deserve a whole iteration on its own — can we remove a bunch of them, for example? On the block library ticket that discusses opening the block library as a modal dialog that defaults to the "Patterns" tab, we made it blue, like so:

Basic Editor

But given it needs more love, and it's holding back this PR, absolutely something to follow up on.

youknowriad and others added 8 commits February 20, 2020 09:55
- Updates borders for dropdowns, and proceeds to do additional polish work outside the editor canvas.
This will no doubt need additional feedback, and the branch should stay open for a while to gather all those thoughts.

One of the primary reasons for reducing the borders is, similar to tertiary buttons, that they work due to their context, just like how menu ite
ms work in their menu item contexts. This puts a great onus on the implementer to choose the right button for the right context. In that vein, w
e may even want to provide additional buttons, such as a solid background button that isn't primary, for cases where a primary button is insuffi
cient, and a secondary button does not work well enough in the context. An example could be considered the link dialog mocked up here #20007 (comment)
This moves the flashing footprint indicator to only the paragraph block.
@youknowriad
Copy link
Contributor Author

Thanks for the updates @jasmussen that's great

By making the inserter a primary, ironically, it also made the sibling inserter primary, which meant I had to undo some of those visuals to make it appear not primary:

I believe we need to have a prop in the inserter to choose whether we want a primary button or not to avoid having to revert the changes in CSS. For example in appenders... I'll see if I can make this change.

Jon Q and others added 2 commits February 20, 2020 10:00
- Restore gray sidebar header.
- Use isPrimary for toolbar library button.
- Make inbetweenserter not look primary.
@youknowriad
Copy link
Contributor Author

Now, that this branch is very close to G2 and without touching the panels... I think we can merge it with G2 and continue on a single branch. Nice work @jasmussen

@youknowriad youknowriad merged commit e167dbf into try/g2 Feb 20, 2020
@youknowriad youknowriad deleted the try/g3 branch February 20, 2020 09:24
@jasmussen
Copy link
Contributor

ƪ(˘⌣˘)┐ ƪ(˘⌣˘)ʃ ┌(˘⌣˘)ʃ

youknowriad added a commit that referenced this pull request Feb 24, 2020
Co-authored-by: Joen Asmussen <joen@automattic.com>
Co-authored-by: Jon Quach <hello@jonquach.com>
youknowriad added a commit that referenced this pull request Feb 24, 2020
Co-authored-by: Joen Asmussen <joen@automattic.com>
Co-authored-by: Jon Quach <hello@jonquach.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Accessibility Feedback Need input from accessibility Needs Design Feedback Needs general design feedback.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants