Add lock feature for Reusable Blocks to protect inner blocks #32710
Conversation
|
Hi, @thisissandip Thanks again for working on Reusable Block issues. I just wanted to mention a general discussion issue on "Locking" - #29864. |
|
Here is a comment made by @cpsyctc "Surely if a reusable block has already been used in any other page/post it should be locked and you should be prompted if you try to edit it and warned that any edit will immediately change all other uses of the block? I would expect a dialogue to come up asking if I'm sure I want to edit the reusable block and offering to convert to an ordinary block." |
|
@thisissandip While this does prevent accidental edits by clicking into the inner block it will not prevent tabbing into the inner blocks and/or accessing them via the document block list view. If you also "locked" the block by wrapping it in the The discussion in #29864 is promising but this is a good interim solution to prevent accidental edits which have been a substantial annoyance amongst editorial teams. |
|
Using the disabled component is much better than using the z-index. lockFeature.mov |
|
A great deal of design thought has been put into using click-through for reusable blocks: #29337 — has that been considered in these explorations? |
|
The toolbar will look similar to this example.
Example from #29864 |
Using a dialogue box to come up every time while editing/unlocking the reusable block might be annoying to the users who know what reusable blocks are. I think the locking of inner blocks will indicate to the user that they are editing a reusable block. Also, while the reusable blocks are saved, users are notified about the changes/warnings in the sidebar.
P.S. after PR #32464 is completed. They can even learn more about the reusable blocks using the welcome guide. |
|
Hi @jasmussen Joen.
The answer to that is that additional explorations should be done. I am thinking that Addison @Addison-Stavlo will likely have some feedback on this PR. I have considered my last test of the click through approach here: #31109 (comment) Transferring above steps over to this lock/unlock PR: |
|
Agreed about the prompt, we don't want to overshoot the target. That's also why I asked about #29337 from February, which was created to solve the exact same problem but with reduced friction. As much as I appreciate the work going on here — at this juncture I can understand wanting some temporary solutions in place for 5.8 — but it'd be good to consult with some designers. Jay is on vacation, but @critterverse or @javierarce might have some thoughts as well. Clickthrough, I believe, is ultimately where we want this to go. |
|
Hey, @paaljoachim! The Block Icon and Block Title in the parent toolbar is a single component that is used to toggle the block type. It is not possible to add a button between them. This is the reason I added the lock option before the convert to regular blocks button.
|
I'm not sure what real world users research was done upon but I can assure you that in an editorial newsroom setting with 100s of users, that clickthrough to inline edit a reusable block is a terrible idea. For example, an SEO editor makes a topical reusable block surrounding a current news event and the editorial team are asked to use it in all relevant articles, but then someones fudges it or doesn't understand its purpose and edits it inline whether accidental or not. They then hit save/publish as they have always done and that change is then propagated across 100s and 100's of articles. It's reckless. The counter argument may be that users are notified about the impending reusable block save in the sidebar when they hit save/publish but in reality the editorial teams haven't clocked it or have just continued onwards regardless with saving without being aware that are about to unintentionally 100s of articles. Authors have deadlines and when we're dealing with breaking news speed is of the essence and UI that blends in with the rest of the page furniture just isn't enough to make users aware. Therefore the locking of reusable blocks safeguards us against these mistakes. Yes you could highlight the severity of what the user is about to do better at the point of saving but the easier way to address this is to prevent the edit in the first place. Since 5.7 we've had to address these issues ourselves because it was becoming a mess to manage both editorially and via the support requests we were getting put in. We've had to filter the reusable block edit method to wrap it in a disabled component to prevent the bulk of inline edits, added a title bar to reusable block that clearly states it's a reusable block and have additionally written apiFetch middleware to short circuit any PUT requests to the blocks endpoint. For the most part we've reworked reusable blocks to prevent the majority of our users from being able to create or edit them inline. In a nutshell, we are unable to trust the integrity of reusable block in their current 5.7 guise when it's far to easy for any user with an edit_posts cap to unintentionally botch 100s, if not 1000s (in some use cases) of articles |
|
Hey all! Thanks for exploring this lock/unlock functionality. From scanning a number of open issues/existing explorations, I'm getting the sense that:
Because of these factors and where we are currently in the release schedule, I'm not sure it's something that's ready for inclusion in 5.8 😬 I agree with @jasmussen that the clickthrough approach in #31109 is a great candidate for introducing some more friction to editing. This approach has been deeply iterated upon by myself and other contributors for several months now so I believe it's a bit more complete in its thinking. In my opinion, the clickthrough certainly doesn't resolve all of the known issues with the Reusable block but it's a big improvement over where we are currently! |
👋 Hiya! Its nice to see more explorations on preventing extra-contextual content from being edited by mistake. I think there are definitely some tradeoffs to each approach (lock vs. overlay). I think the overlay has a little less friction and is a bit more visible in the editor. However, if more friction is needed in list-view and keyboard-nav flows a locking system like this would definitely help with that. I would think its best to start on the side of less friction and move this direction if the former isn't enough.
@hrkhal - From your description I think you may be misunderstanding what was meant by 'clickthrough' approach (thats understandable, as the name itself kind of implies you can just click through to edit with no safety steps in between). The idea here would be to prevent users from selecting children (content of the reusable block) without first selecting the parent. When the reusable block is hovered in editor or list-view, a semi-transparent colored overlay would appear over its content. Clicking the overlay would then force selection to the reusable block itself instead of its children, unlocking it so that its children could then be selected. (explored in #31109) |
|
Can someone define the steps to lock and re-lock the blocks in the clickthrough approach? I will update this PR and then we can test and come to a conclusion that which approach feels robust against accidental edits. Is that okay? From #29337 example video, The steps to unlock are:
What are the steps to re-lock the block? |
Just deselect all blocks.
That's not actually right. A single click selects the top level block, the reusable block itself. Now all child blocks are unlocked and any child block can be selected. Double click as described by Jay in 29337 just means "click twice" — once to select the reusable block, once again to select the block inside. Thank you for looking. |
|
Hey guys! 👋 I have implemented the clickthrough approach for locking and unlocking! If you guys can take a look 👀 . That'd be great! clickthrough.mov |
|
Adding my two cents here: IMO, having a dedicated lock option is more robust and intuitive. For example: In Notion (which uses a similar block-based editing approach), they have a dedicated lock option to lock the page so that no changes can be made unless one clicks on the dedicated unlock button. Another example: In Photoshop and Illustrator, there is an option to lock the layer and it cannot be unlocked unless we click the dedicated lock option again. Imagine while designing something in PS and we accidentally unlock a layer by clicking on it, adding accidental changes. |
|
I would love to help with this as what is in 5.7.2 is in my view very bad and definitely wasted hours of my time until I realised it was so broken I had to wait for it to be fixed.
However, and this is a meta-issue but perhaps a very important one: I can't help with end user thinking if the terminology is opaque (and I think I saw an acknowledgement that
the term "clickthrough" is opaque) and if the screen recording expects me to see what is happening (and certainly not at that pace and with no voice over) then I for one
can't help. Sorry, I haven't a clue whether that fixes anything that I think is broken or not. I simply didn't understand what was happening despite playing it through a number
of times. That does make me wonder how WP sets up testing and can pull end users in to help.
I do realise that is tangential to your focus but I really would like to help and would commit some time to that, I see that as my duty as someone who believes in open source
software, but if it's this incomprehensible to me then perhaps there is a structural, organisational, in some ways cultural or sociological, issue about how to manage improvement.
The only other FLOSS system I do try to contribute to is R and clearly the forces and needs that shape that are very different from those shaping WP, however, I can
see much of the way the huge R project is structured is much more tilted to slow improvement and backward compatibility than that in WP which increasing seems to me to
be moving too fast and too much focused on visual aesthetics and not enough on usability for editors and admins.
OK. Sorry, tangential splurt over. If someone has time to explain what was going on there, I will try to be more useful (but I am on leave 21-28.vi.21!)
Good luck!
Chris
… From: "thisissandip" ***@***.***>
To: "WordPress/gutenberg" ***@***.***>
Cc: "Chris Evans" ***@***.***>, "Mention" ***@***.***>
Sent: Thursday, 17 June, 2021 16:46:48
Subject: Re: [WordPress/gutenberg] Add lock feature for Reusable Blocks to
protect inner blocks (#32710)
Hey guys! I have implemented the clickthrough approach for locking and
unlocking!
If you guys can take a look 👀 and review it. That'd be great!
Thank you for testing and exploring this PR! 😄
[
https://user-images.githubusercontent.com/69596988/122430419-375dfe80-cfb1-11eb-8e31-1782eb04917a.mov
|
https://user-images.githubusercontent.com/69596988/122430419-375dfe80-cfb1-11eb-8e31-1782eb04917a.mov
]
—
You are receiving this because you were mentioned.
Reply to this email directly, [
#32710 (comment) | view
it on GitHub ] , or [
https://github.com/notifications/unsubscribe-auth/ACP3COQC64OIIHCDYHBBVODTTIKGRANCNFSM46X5CJTA
| unsubscribe ] .
--
Chris Evans (he/him) ***@***.***> Visiting Professor, University of Sheffield ***@***.***>
I do some consultation work for the University of Roehampton ***@***.***> and other places
but ***@***.***> remains my main Email address. I have a work web site at:
https://www.psyctc.org/psyctc/
and a site I manage for CORE and CORE system trust at:
http://www.coresystemtrust.org.uk/
I have "semigrated" to France, see:
https://www.psyctc.org/pelerinage2016/semigrating-to-france/
https://www.psyctc.org/pelerinage2016/register-to-get-updates-from-pelerinage2016/
If you want an Emeeting, I am trying to keep them to Thursdays and my diary is at:
https://www.psyctc.org/pelerinage2016/ceworkdiary/
Beware: French time, generally an hour ahead of UK.
|
|
This I do understand and agree with completely. That made me think about locking and unlocking in InDesign (where the unlock granularity was lost way back: to my
mind a mistake but perhaps forcing people, or at least encouraging them, to make far more use of layers. Mind you, the word "lock" there is about position more than
content, for me the issue with reusable blocks is that their content would be safer were it not editable when in place unless unlocked in the sense of being no longer
an iteration of the reusable block. Is there a parallel with themes and child themes? I would expect a reusable block, like a theme, to be changed in all its clones if
changed but I would expect to do that in a "reusable block editor" not where I'd placed it for use.
Sorry, perhaps again I'm just getting tangential and talking about user language not code language, but I suspect this matters.
… From: "thisissandip" ***@***.***>
To: "WordPress" ***@***.***>
Cc: "Chris Evans" ***@***.***>, "Mention" ***@***.***>
Sent: Thursday, 17 June, 2021 19:08:28
Subject: Re: [WordPress/gutenberg] Add lock feature for Reusable Blocks to
protect inner blocks (#32710)
Adding my two cents here:
IMO, having a dedicated lock option is more robust and intuitive.
For example: In Notion, they have a dedicated lock option to lock the page so
that no changes can be made unless one clicks on the dedicated unlock button.
[
https://github.com/camo/2205c7a7a6fb74986563ee21d3819519e8b1fee28c79448b63494bd0b99911e5/68747470733a2f2f73332e75732d776573742d322e616d617a6f6e6177732e636f6d2f7365637572652e6e6f74696f6e2d7374617469632e636f6d2f33383532626434392d313435322d343566372d616361392d3533316464393636333566622f4c6f636b5f506167652e6769663f582d416d7a2d416c676f726974686d3d415753342d484d41432d53484132353626582d416d7a2d43726564656e7469616c3d414b49415437334c324734354f334b5335325935253246323032313036313725324675732d776573742d322532467333253246617773345f7265717565737426582d416d7a2d446174653d3230323130363137543138303534335a26582d416d7a2d457870697265733d383634303026582d416d7a2d5369676e61747572653d6365653130373231336539346139386138343939303939633164383335326161303666633430363362663434616563643366313332333636326233643037663626582d416d7a2d5369676e6564486561646572733d686f7374
]
[ https://www.notion.so/Lock-page-content-d2b995727c0b483f9f3518c1eb4fab75 |
Source ]
In Photoshop and Illustrator, there is an option to lock the layer and it cannot
be unlocked unless we click the dedicated lock option again.
Imagine while designing something in PS and we accidentally unlock a layer by
clicking on it, adding accidental changes.
—
You are receiving this because you were mentioned.
Reply to this email directly, [
#32710 (comment) | view
it on GitHub ] , or [
https://github.com/notifications/unsubscribe-auth/ACP3COUZULFYKVGJUKGRYRTTTI2ZZANCNFSM46X5CJTA
| unsubscribe ] .
--
Chris Evans (he/him) ***@***.***> Visiting Professor, University of Sheffield ***@***.***>
I do some consultation work for the University of Roehampton ***@***.***> and other places
but ***@***.***> remains my main Email address. I have a work web site at:
https://www.psyctc.org/psyctc/
and a site I manage for CORE and CORE system trust at:
http://www.coresystemtrust.org.uk/
I have "semigrated" to France, see:
https://www.psyctc.org/pelerinage2016/semigrating-to-france/
https://www.psyctc.org/pelerinage2016/register-to-get-updates-from-pelerinage2016/
If you want an Emeeting, I am trying to keep them to Thursdays and my diary is at:
https://www.psyctc.org/pelerinage2016/ceworkdiary/
Beware: French time, generally an hour ahead of UK.
|
|
Nice exploration @thisissandip 👏 I just wanted to flag that we should try to be conscious about creating duplicate PRs here as we already have one that achieves the "clickthrough" functionality for Reusable blocks quite well in #31109 :) @cpsyctc I'm not sure whether this completely answers your questions but I thought @paaljoachim explained the usefulness of the clickthrough approach really well the other day on Slack: https://wordpress.slack.com/archives/C02RP4X03/p1621618806126700 Here's his comment in case you can't view the link:
|
|
I am adding in a threaded Slack conversation during the Wednesday Core Editor chat: https://wordpress.slack.com/archives/C02QB2JS7/p1623855296191000 @jasmussen @paaljoachim Carlos Garcia Prim |
|
Brainstorming. Here is the newest version of the Click through approach in this PR: #31109 (comment) 1- Hover over a Reusable block and notice the overlay. Click-through-Reusable-block.mp4Various blocks that have inner content such as Buttons, Social Icons, Columns, Groups and other blocks could benefit well from using the Click through approach.
Reusable blocks are more vulnerable for editing because of the nature of a Reusable block. It is meant to be a block that can be added to multiple pages or posts. Editing any of these instances of the Reusable block will also affect all other instances where it is used. Which means either one creates a stronger protection such as unlocking a lock (ex. in WP 5.6 clicking the Edit button) or one adds a protective measure to when the block is saved. Making it harder forcing the user to make a deliberate choice. It is better to add a protective measure into the editing state 1- Hover the block and see the overlay (similar to the Click through approach) + See a lock. Perhaps something like this: Reusable-blocks-with-click-through-and-a-lock.mp4Adding a lock says this is something that should not be edited lightly. |
hey @paaljoachim! 👋 Should I update this PR with the second option ( light overlay with a lock )? |
|
I'd love to give this branch a run tomorrow. I don't think we need locks. |
|
Thank you again for working on this, @thisissandip. Your patience and work here is much appreciated, it's been inspiring to see. Here's what I see in your branch:
This is impressive work, and I think validates the idea of the overlay color, and the extra click. It also lets us avoid complex accessibility scenarios where you have to tab backwards to get to the lock, then forwards again to get to the block you meant to edit. In the clickthrough scenario, mouse and keyboard are equals. The only downside is that puts this branch in walking distance of #31109, which does the same but for template parts as well. To Paal's point, we might want to land that first, and then look at what improvements we need to make after that fact. Do we need further friction? My sincere instinct tells me no: any changes you make aren't applied until you've had a chance to confirm them in the publish dialog, and the emphasis on that dialog might give it the separate attention it needs. By starting small, we can carefully add improvements to safeguard against accidental changes, such as limiting the editing of reusable blocks to certain user roles, perhaps only to the original author or administrators. If after testing that in the plugin for a bit and we find the need to add an extra click to unlock, we can revisit how to do that without affecting the tab-order, and we can revisit this one. By trying one before the other, we'll better know what improvements we can get into 5.9. |
There was a problem hiding this comment.
Thank you for the explorations and discussions going on! I'm not sure of what path will be taken on but as promised to @paaljoachim I'm leaving a code review here. Generally from the code point of view things seem good 👍 I just left some logic questions.
|
|
||
| const currentBlockId = getSelectedBlockClientId(); | ||
| const parents = getBlockParents( currentBlockId ); | ||
| const _firstParentClientId = parents[ parents.length - 1 ]; |
There was a problem hiding this comment.
I guess if the reusable block is used inside a columns block for example this logic would not work?
There was a problem hiding this comment.
Yes, it does work when reusable blocks are inside columns.
|
|
||
| // lock the blocks when deselected | ||
| useEffect( () => { | ||
| const isInnerBlock = parentBlockName === 'core/block'; // first check if selectedblock is inner block |
There was a problem hiding this comment.
We check if the block is inner block by block type and not by verifying if the specific reusable block id is the parent of the deselected block, if we have multiple reusable blocks on the page will this logic handle that case?
There was a problem hiding this comment.
Thank you for taking a look 😄
Yes, it does work when multiple reusable blocks are on the page because first we have to select the reusable block itself, and only after that, we can select the inner blocks.
So, when multiple reusable blocks are on the page, after working on the first reusable block, we go and select the other one. Now, first, we have to select the outer block (reusable block itself) which won't have a parentBlockName of core/block thus it will lock the reusable blocks. The same logic follows in the case of the reusable blocks inside columns.
The bug I came across while testing this was when a reusable block is selected and its inner blocks are not selected right after that but another reusable block is selected.
In such a scenario, the previously selected reusable block remains unlocked.
I am adding a commit to fix this bug. Once again, thank you for reviewing it! 🙏
|
@thisissandip Sandip it would be great to use the learning from this PR in creating a click through approach and then look at @Addison-Stavlo 's PR and do a code review etc there. As we should first get that PR merged. Test it out for a while and then see how your PR here can be brought forward. |
|
@paaljoachim Yeah sure! 😄 |
annezazu
added
the
[Feature] Synced Patterns
|
Hi everybody. After using Reusable Blocks for a very long time on many projects. Today, I got finally so annoyed by editing them accidently, that I looked for a solution to lock them from accidental editing while editing another post. I'm using them over 2 years daily, and I want to give you the feedback, the implemented solution in the PR 31109. is not sufficient to stop editing them. Until I've read it here, I even didn't understand, that the clickthrough approach was implemented in 11.0/5.8.2. My thought process from an editor perspective is more like that the first click "didn't work". I strongly want to suggest looking again into the dedicated locking feature with a button in the toolbar for Reusable Blocks. I would help with it, but I'm not good enough in JavaScript to actually understand the Gutenberg code. Thank you! |
|
Closing now that #39950 is merged. |
Description
Fixes #32461
Add lock option for the Reusable Blocks in the parent toolbar.
Protects the inner blocks from accidental edits.
Unlock Icon is not available in @wordpress/icons package. So as of now, I have used plain text to display the lock and unlock state in the toolbar.
How has this been tested?
Screenshots
LockAndUnlock.mov
Types of changes
New Feature
Checklist: