Closed davisagli closed 1 year ago
Name | Link |
---|---|
Latest commit | 10422e726b7d017a00f378489e2d875e1af6ceb1 |
Latest deploy log | https://app.netlify.com/sites/volto/deploys/6377adea5e891c00087fa3ab |
@davisagli looks good and solves the problems highlighted in #682. vs the notion way of doing it I suggested in #682 there is not that much difference so I fully supporting going ahead with this solution. Differences are
This is a vast improvement. I like the appearance of only the active block's +
and the last block's +
, or if they happen to be one and the same, then just the one.
I also noticed that when a user hover overs a given block, its outline appears, but not its +
. Adding a +
on hover would save a click. Instead of click to activate, then click to add block, one could hover and click the +
to add a block. It might be tricky to implement, though, and I think this improvement is good enough as is. I would not extend this one-fewer-click thing to drag or delete, as I think those actions should be more deliberate.
Adding a + on hover would save a click. Instead of click to activate, then click to add block, one could hover and click the + to add a block.
I'm open to trying that, but was trying to avoid hover effects since they don't work well on touch devices.
Adding a + on hover would save a click. Instead of click to activate, then click to add block, one could hover and click the + to add a block.
I'm open to trying that, but was trying to avoid hover effects since they don't work well on touch devices.
I think it's fine since its a progressive enhancement as mobile users would just click.
I also don't see a problem with being able to drag or delete from hover. There is undo and I can't see it being done by accident that easily.
I don't feel strongly for or against the idea of showing the buttons on hover, but it'd be significantly more work and I don't want to add it to the scope of this PR.
There is undo and I can't see it being done by accident that easily.
OMG! Why is this not in the docs?
https://6.dev-docs.plone.org/search.html?q=undo&doc_section=volto
I guess because there is no end-user Editor documentation for Volto? I have an idea here: https://github.com/plone/volto/issues/3811#issuecomment-1298030528
@davisagli
Amazing work! Initial Pastanaga designs were very close to this, but every time we tried it out seemed quite cumbersome and intrusive (as in adding too much space between blocks). The compromise you are using in your implementation looks fine for me. I'm only a bit worried about discoverability, like it does not stands too much from the rest as the position it occupies is always crowded visually. However, not a problem for a user that already knows how editing work in Volto.
As @tiberiuichim said, it's good that we are already moving it where it belongs too, in preparation for Quanta I think overall is good.
I'd be even +1 to add it to 16. What do you think? I'd like to hear more opinions. /cc @plone/volto-team
I'm +1 on merging it ASAP.
Yes, I would definitely add it in 16
@sneridagh I'd say it has to go into 6.0. The first time user experience is very important IMO.
@davisagli before this is merged I think there needs to be a change to take care of the case where no blocks are left on the page. The assumption that the title block will always be there only holds for the out of the box experience Page type and the first thing anyone does is create a landing page type that removes the title so you can use something like a hero block instead. As you can see in the example below I can delete the top block and therefore every block
my 2c opinion on how to fix this would would still be use the text cursor and clicking in the blank space below the last block (or no blocks) to create an empty text block. This is something that can be added in addition to the + at the bottom of a block and solves the zero block problem. I have less concerns that discoverability is an issue than @sneridagh since users tend to click on things when they want something to happen in that space. its natural. But I guess a permanent + is ok too to solve the zero block problem.
I'd say that if we can solve it, let's find it out (Quanta has the final + always in place).
However, we had this situation since day one in Volto and we had no complaints reporting this as feedback from our clients. Although certainly a possible use case, it's something that could happen quite unlikely. There are in place also worksronds for it (undo).
But as I said let's think about it. We have to not make tthis a blocker either, once we have an acceptable solution for the rest.
@davisagli Wow. thank you so much. This looks a lot more intuitive for first time or occasional users.
The only inconsistency / edge case I still see is with current Volto behavior. The original (+) button was never an add button but a change/replace button: it changed the fresh empty text block that was added through hitting 'ENTER' in the previous block. Now that the + is a real 'add' in this proposal, it would still be useful to have 'change' button or some other mechanism where empty inserted text blocks (or placeholders) can be changed into another type of block.
For existing users that have the" Enter for a new block" in their muscle memory, you could open the block selector automatically when giving a second enter. And mouse users clicking the center (+) already operate the mouse and naturally continue selecting the block type. (Is the block selector keyboard navigable? )
But there is still the edge case of an empty (previously) created text block which the editor wants to change, and now there is only the (+) that correctly adds a new block below, but the change button is gone.
My gut feeling on the (+) being already there on hover to save a click is to hide it andkeep the UX cleaner and simple to remember. first click always selects a block, second click is an action (edit, add). But as Steve piercy writes it does save an extra click in mouse operation. Undecided.
Definetely a very good onboarding polish to add to Volto 16 if we can make it happen.
@djay I agree we should look for a good solution for when there are no blocks. I'll take a look at that.
@fredvd Actually the existing (+) button is not a change/replace button. I just tried it on 6.demo.plone.org and it inserts the new block above the empty text block, without removing the empty text block. I didn't show it in the video but my implementation works like this:
I agree the block chooser should be accessible by the keyboard, but I'm not sure what is the best shortcut to use.
Another hidden volto feature :) On an empty text block, type a slash (/), you'll get a dropdown with available blocks, which you can navigate via keyboard
@tiberiuichim I was a bit confused why the /
dropdown is different from the main block chooser. I guess that's intentional to optimize for keyboard navigation?
Notion makes this feature less hidden by having a empty block hint of "/ for commands" which is a good idea.
They also make it a shortcut you can use anytime by allowing you to use it anywhere in a text block. At the start is insert before. Anywhere else is insert after. And empty block it replaces the current block.
It don't see why we couldn't copy all of that behaviour.
@davisagli Yes, it's optimized for keyboard navigation. Actually, I've omitted the part that it also acts as a filter. So if you type "/im" you'll get a dropdown with only two entries, "Image" and "Images grid" (on 6.demo.plone.org)
In general, I like the idea. Though, I'd like to show this to our UX experts (@albertcasado et al) to get feedback. That is quite a fundamental change in how Volto works and it definitely needs careful consideration and a major release. We have to take into consideration that our current users will be confused by the change and that our clients will need to amend all their documentation and screenshots.
@tisto Can we get that feedback quickly? I agree it's a change with some impact, so all the more reason to try to get it done before 16 is released rather than wait a long time to fix this usability problem, in my opinion.
@davisagli I showed the new implementation to one of our UX experts (in our current main project :)) and his feedback is that overall he thinks this is an improvement. He had two things that we could improve:
Do we have the new implementation shipped somewhere where we can point people to toy around with it?
Though, maybe we just want to show the + on the last block if no other block is selected
We already handle this case, we have code that ensures that there's always an empty placeholder block created at the bottom. To test: 6.demo.plone.org, create new Page, on the text placeholder block, click + to open Block Chooser, select Image block. A new placeholder block will be created at the bottom.
We already handle this case, we have code that ensures that there's always an empty placeholder block created at the bottom. To test: 6.demo.plone.org, create new Page, on the text placeholder block, click + to open Block Chooser, select Image block. A new placeholder block will be created at the bottom.
@tiberiuichim We've found this to be a bit of a problem, because editors forget to remove the empty block at the end, and it messes up the spacing before the footer. In my implementation, I removed it because now the add button is available on any block, so the empty placeholder block doesn't seem necessary.
the plus symbol shows up in the block that is currently selected and in the last block, this might confuse people. We used to show the last plus to indicate to editors they can add something. Though, maybe we just want to show the + on the last block if no other block is selected
I'll try this.
the drag handle currently does not show up on an empty text block. This is because we used to show the + icon there. We should amend this as well
Good point.
@davisagli throw-back to an old ticket, https://github.com/plone/volto/issues/1851
Here is a demo site where you can play around with the new add button: https://plone-frontend.onrender.com/
This is running on render.com's free tier, so the instance shuts down after 15 minutes of inactivity. It will probably time out when you first try to visit it, but should work if you try again after a minute or two.
The demo includes several of the suggested improvements from this discussion:
There's a zindex problem with the block chooser.
I think the + on the left side in current Volto is "stranger" then the proposal here, we're just used to it being there. I think the this proposal is good, it solves UI problems for volto. Let's hear more feedback.
@tiberiuichim Interesting, I can reproduce the z-index problem but don't understand it. It happens when I open the chooser between two image blocks, but not when I open it above both of them. The problem seems to go away if I disable this CSS rule: .ui.drag.block.image { z-index: 2; }
CSS shenanigans
@davisagli take a look at the pr that brought that in. I don't remember why it's there, but I can imagine it's because of something.
I'll also try how the nested blocks addons behave with this change
@sneridagh note the demo site is deployed from my fork which has a few changes that aren't in this PR yet: https://github.com/davisagli/volto/tree/add-block
It's amazing how a small change makes a huge difference. This is very useful. Thank you!
It looks good to me, one thing though
- I added a feature to detect a click in the empty area below the last block, and add a new text block (same effect as hitting Enter while the last block is selected). This also solves the problem of how to add a new block if the page is completely empty.
The text cursor appears all around the editor area, if you click on a side it does nothing, but if you click above the title, it also creates the empty block at the bottom.
I'd like to discuss this next week in the Volto Team Meeting. We could reschedule it to a time that @davisagli could attend too. /cc @plone/volto-team
The text cursor appears all around the editor area, if you click on a side it does nothing, but if you click above the title, it also creates the empty block at the bottom.
@sneridagh Fixed; now it should only add a block when the click is actually below the bottom of the existing blocks. But I haven't thought of a good way to only show the text cursor there. That seems like a pretty minor issue to me.
Project | Volto |
Status | Passed |
Commit | 10422e726b |
Started | Nov 18, 2022 4:11 PM |
Ended | Nov 18, 2022 4:23 PM |
Duration | 12:21 💡 |
OS | Linux Ubuntu - |
Browser | Chrome 107 |
View run in Cypress Dashboard ➡️
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard
First of all: This is a great enhancement!
I think it is important, and you do it this way, to show add buttons only on the selected block. Otherwise add buttons on every block would be a distraction.
I assume it is out of scope of this PR: For blocks like @eeacms/volto-columns-block
i would like to have the same behavior: show "+" button on bottom border if a sub block is selected.
@ksuess volto-columns-block has its own EditWrapper, similar to what David added here, so we need to add the button in there.
For the sake of completion, this was the initial mockup from PastanagaUI:
@davisagli There is an issue with TTW DX Layout editor. When you press Enter all restrictions on that block are wiped out. See:
@davisagli There is an issue with TTW DX Layout editor. When you press Enter all restrictions on that block are wiped out. See:
@avoinea I think this is a separate issue. I can reproduce it also on https://6.demo.plone.org/, and my work here doesn't make any changes to what happens when Enter is pressed.
@davisagli I see
Just to give a bit of an update here... Sorry for the radio silence; I've been focused on another topic at work and was on vacation over the weekend, so I haven't had time to get back to this PR yet.
In general it sounds like there is a lot of interest and I intend to keep working on it. I am also planning to take a look at making the new add button controlled by a (temporary) setting, so that there is a bit more flexibility in how it gets rolled out for existing projects. #3896 explains the reasoning for this more.
I think I've addressed all the feedback (or responded why I think it's out of scope) and it's ready for final review.
@davisagli
While I agree experimental feature flags is a good idea and does provide a way to move things forward for some UX features my 2c is that this shouldn't apply in this case. I believe this change should be turned on by default for everyone and perhaps have a way to disable it if required.
Why?
1) its a bug fix not a feature.
2) Marketing volto as intuituve
3) it's a non breaking fix.
4) this helps fix mobile editing
Maybe we can have 2 flags? an experimental feature flag and a pre-6.0 UX flag?
I think it should be on by default for plone 6.0 and have the ability to turn it off if desired for the sake of not upsetting existing users
@djay This is exactly how I've already implemented it, so I'm confused why you've requested changes.
In general I think new experimental features should start off by default, but I think this one should be an exception for the reasons you covered.
@djay The proposal is one flag per feature, not a single flag that turns on all the experimental features.
@tiberiuichim github shows that you requested changes a while ago; could you update your review?
@sneridagh Can we merge this for Volto 16?
I discussed this PR this morning with @tisto, @ericof, and @sneridagh. Timo is still concerned about merging it with the flag turned on when it hasn't been tested in any actual projects.
So, we plan to merge it with the flag turned off, to make sure it's included in the release candidates. We intend to switch it to be on by default by the time of a final release of Volto 16 as long as we hear positive feedback from a real project.
@djay I know this isn't quite your ideal, and I agree with your arguments for why it should be on by default. But I think it's the path we need to follow to actually get it into Volto 16 while keeping everyone happy. It should work out the way you want unless there is some unanticipated surprise.
@davisagli I totally understand the need for caution. It's not an ideal situation to be introducing bug fixes so late. However if the intention is to have it on by default I don't understand the idea of turning it off now? Only a few will know to turn it on and test. Surely the only way to know if there are problems is to have as many people use it as possible? Otherwise we will get to the end of the RC and still be as uncertain about turning it on as we are now?
Here is a prototype for a more intuitive add block button, which fixes #682. Here's a screencast showing how it compares to the status quo (sound on):
https://user-images.githubusercontent.com/49014/199147930-dd0964bb-ca3b-42b8-a349-10efd539efef.mov
Details:
I'm interested in feedback on whether this is desirable and whether there are edge cases I overlooked. If people are generally in favor, I'm happy to continue the work to update the tests and such.
(I'm aware that the Quanta design also includes a new block add button. This is my attempt to borrow the best parts of that idea without tying it to the rest of Quanta, and also to think through some details that aren't covered in that design like how to make the button work in between multiple blocks without messing up the spacing.)