Open klonos opened 4 years ago
This issue here is also related to #6242. It would be much more useful to be indicating where else the block is being used + throwing a notice to let people know that they are updating the content of the block everywhere.
This is a good point. It is both an advantage to know that changes to a reusable block apply everywhere it is used and could be a problem if the user forgets that. Personally, I prefer reusable blocks (like reusable data) to avoid clutter. Possibly something on the admin/structure/block/list page could show where the custom blocks are used. This would be similar to the approach used by the imageblock module.
OK, I've played around with the various places in the UI, and here's a summary of where I'm at with this. ...I'll use some side-by-side before/after screenshots, which explain the changes in a better way, but feel free to spin up a demo of the current version of Backdrop yourselves, and compare that to the respective pages/forms in the sandbox of this PR: https://github.com/backdrop/backdrop/pull/4532
Before/after when adding a new custom block (the only changes are in the checkbox label and help text):
Make this block reusable
becomes Make this block reusable across different layouts
, which is slightly longer, but the total text (label and help text) that we use to describe this functionality is reduced that way.How reusable blocks work
"teaser" + a show/hide toggle. This keeps things neat and the form much less cluttered. Once someone expands the help text the first time and reads the various notes/explanations of what it means to make a block reusable, they don't need to keep seeing the help text every time they are creating a custom block. If they need a refresher, they can use the toggle to expand the help text, read it again, and then hide it. This is a similar UI pattern as the one we have used in the layouts listing page, to hide the lengthy help text that was necessary in order to explain how layouts and their priorities work etc....
...the above is when creating a new custom block. Next, here's what before/after looks for existing custom blocks (the issue that triggered the creation of this ticket in the first place):
So basically, once a block is made reusable, we don't show a disabled checkbox any more - we show an info message.
I thought that a warning would not have been appropriate, since updating block content in one place and having it be updated everywhere that block is used is not a bad thing - just somethings that people should be aware of. So I just bolded that bit to stress that out.
The info message basically includes the same text as the 3 bullet points from the help text of the button, but not as bullet points (tried that, but the top of the form seemed to be too busy that way).
...next, here's a before/after of the admin/structure/block
listing:
The only change here is the addition of the Updating the content of these blocks in one place will affect all places where they are being used!
text at the top of the page.
PS: I think that this page should be renamed to "Reusable blocks", since it is not listing all custom blocks - only the reusable ones. But that would be a topic for a follow-up issue.
...next, here's what editing a custom block via the operations links available in the admin/structure/block
page looks like before/after:
The only changes are:
The title of the block as shown to the user. This will affect all places where this block is used.
Optional block title to be shown to site visitors.
The This will affect all places where this block is used.
is already mentioned in the message at the top of the page, and instead of using the word user
, we're now using the term site visitors
(which is what we are already using in the help text for the "Admin label" field)....finally, here's what the confirmation form when deleting a reusable block looks like before/after:
PS: this is another place where it would be beneficial to be listing all layout/regions where the block is being used. Again, better for that to happen in #6242
Please let me know what you think of the UX before/after and if you have suggestions for wording changes. Also let me know if all these changes should be split into different issues in order to discuss them separately. When I was working on this, it felt more like them belonging together, and perhaps reword/repurpose this issue (the PR is already titled [UX] Improve the UI around reusable blocks
).
Very nice. I think these changes should be kept together, there is no reason to split them up. I wondered if making the warning sentences BOLD was too much, but I don't think so.
The delete option could include mention that alternatively the block can be removed from individual layouts.
I wondered if making the warning sentences BOLD was too much, but I don't think so.
Ha, I wondered also if the bold parts are too noisy, and in my opinion they are, at least in the info messages. (The bold text in the warning message is okay.)
@olafgrabienski I agree with you. Removing the bold from the info messages and leaving the bold in the warning messages would be ok.
Thank you both for the feedback @olafgrabienski and @izmeez 🙏🏼 ...do you have thoughts on other aspects of the UI and the changes in the PR? Any thoughts on wording? For instance, I liked this suggestion:
The delete option could include mention that alternatively the block can be removed from individual layouts.
I was thinking to add something along these lines (needs wordsmithing):
If you intended to remove the block only from a specific layout, then you can edit the layout in question, and use the "Remove" or "Disable" operations, or adjust the visibility conditions of the block for that layout.
I'll wait for a few more people to chime in and address any additional feedback in one go.
A slightly shorter version:
To remove a block only from a specific layout, edit the layout in question and use the "Remove" or "Disable" operations, or adjust the visibility conditions of the block in that layout.
I thought the rest of the UI changes were good, only questioning the amount of bold.
If you really want knit picking to consider:
On the admin/structure/block
page:
Remove the word "being" so it reads "...all places where they are used!"
In a layout when configuring an existing custom block, the warning message can be adjusted by removing "You may update ..." and replacing with "Its content may be updated either ..."
I would agree that there is a bit too much bold going on. I also noticed this:
If memory serves me correct, we try to avoid providing links that would take someone away from a page that might be incomplete and where they might loose changes.
@klonos Do you want to rebase this and create a new sandbox or should I create a new PR and try to address some of the changes mentioned here?
Thanks for the feedback/suggestions @izmeez and @stpaultim 🙏🏼 ...working to update the PR.
OK, PR updated:
t()
functions to use url
instead of l()
(see https://www.drupal.org/docs/7/api/localization-api/dynamic-or-static-links-and-html-in-translatable-strings).target="_blank"
to the links, so that there is no form data loss if the link is clicked (thanks for catching that @stpaultim 🙏🏼 ).Note:
prefix to the text, and have bolded only that one word instead. I hope that this is an acceptable middle ground.#field_prefix
to move that message directly to the "Block content" field, between the label and the field itself. It feels more in context there. Let me know what you think about this change.backdrop_set_title()
with check_plain()
when the PASS_THROUGH
option is used (we were doing that in order to display italicized names of things).Here are some before (PR before this commit)/after screenshots:
admin/structure/block
page (shorter message, and using "will be reusable" instead of "is reusable"):
Despite my experience with Backdrop CMS, I like the fact that these changes are teaching me things about blocks and reusable blocks that I didn't understand or was confused about. My concern about the PR is a possible overuse of the info messages and/or too much text in the one warning messages included in this PR.
I just had a 30 minute discussion with @klonos in Jitsi to clarify some of his choices in the PR and talk about ways of tightening this up a bit.
We focused on two things:
Looking at this issue, inspired me to open another issue that is related, but might be easier to implement quickly as work/discussion continues on this PR. I discussed with @klonos if my suggested changes should be part of his PR or addressed seperately and we decided to start by opening a new issue and see how that goes.
Thanks @stpaultim 🙏🏼 ...I'll work on the PR a bit more, to address the feedback you provided and the things we discussed.
In the meantime, any other thoughts/feedback from others is greatly appreciated.
@stpaultim in https://github.com/backdrop/backdrop-issues/issues/4108#issuecomment-2106032655
We focused on two things:
The two images are really hard to read.
The two images are really hard to read.
Those images are the right side of the following images, already shown in a previous @klonos' comment.
Thank you for referencing the larger images. The images on the right side are definitely an improvement.
PR updated:
(reusable)
suffix to the "Block content" field label. I have found this to be making a huge difference, as it removes the need for the This block is reusable across different layouts
portion of the text from the info message.Tip:
prefix to it, to make it subtly stick out a bit - let me know if that doesn't look good. This help message is only shown for reusable blocks, and only in the "Add block" dialog - it is not shown when editing/creating standalone custom blocks via /admin/structure/block
.Note: The content below will be shared across all layouts where this reusable block will be placed.
Note: Any changes to the content below will affect all layouts where this reusable block is placed!
Deleting this reusable block ...
text completely - the new text in the delete button makes it clear what the consequence of the action will be.This action cannot be undone!
text as warning.Tip:
prefix).Screenshots:
/admin/structure/block
(note the (reusable)
suffix):
/admin/structure/block
(slightly different text on the note):
Eager to get feedback on this new iteration of improvements!
@klonos - I think this is a nice improvement. Much more effecient, but conveys important information and helps people avoid mistakes. For me, this PR is pretty close to WFM.
I have a few thoughts on this specific item and am anxious to hear what others think.
I'm still a little nervous about the link here. I know that you added the "target new tab" link to avoid content loss. But, is that something we do anywhere else? Are we sure we want to be opening new tabs for something like this? I'm not 100% against it, am curious what others think and/or more info about whether or not we do this anywhere else?
I don't think we have "Tips" anywhere else. Also, not sure that is a bad thing. Maybe this is something we will do more of in the future. I like the idea of having "tips" spread throughout the site and I believe that we have other issues that discuss this. It would be nice to have a settled format for this. But, I think we could move forward with this, as it is, and settle the bigger question later.
Finally. I think we could tighten the language a bit more here. By the time someone is reading this, I think it's pretty clear that they can edit this block here. I'm not sure we need to tell them that. Maybe this line should just be:
Tip: You may also edit this and all other reusable block on the Custom blocks page.
I'm still a little nervous about the link here. ... is that something we do anywhere else?
This might help you get an idea @stpaultim: https://github.com/search?q=repo%3Abackdrop%2Fbackdrop%20_blank&type=code
I don't think we have "Tips" anywhere else. ... Maybe this is something we will do more of in the future. I like the idea of having "tips" spread throughout the site ...
We could standardize this (follow-up?), but we do have similar patterns where we partially blod text that we want to draw attention to. Some random examples:
Finally. I think we could tighten the language a bit more here. By the time someone is reading this, I think it's pretty clear that they can edit this block here. I'm not sure we need to tell them that. Maybe this line should just be:
Tip: You may also edit this and all other reusable block on the Custom blocks page.
Agreed 👍🏼 ...I've updated the PR with that.
Thanks for the great work here, I like the direction!
Note: Any changes to the content below will affect all layouts where this reusable block is placed!
Unfortunately, this isn't only valid for Block content but also for the Display title if the Block title type is set to "Default". I guess, this makes the issue even more difficult.
@olafgrabienski Do you think your conern is a blocker? Would it help to move this info message up above the Display Title?
I'd be inclined to mark this Works for Me, except for this outstanding question.
Unfortunately, this isn't only valid for Block content but also for the Display title if the Block title type is set to "Default". I guess, this makes the issue even more difficult.
Hmm, yeah, that'd be a lot trickier @olafgrabienski 🤔 ...we could be adding the (reusable)
suffix to the label of the title, but it would need to be done conditionally (only when the value of the select is set to "Default"). #states
does not allow that at the moment though. I'd be happy to file a follow-up if you are OK with that. In the meantime, I wouldn't want to block the rest of the UI/UX improvements here based on something that cannot be done easily currently.
Do you think your conern is a blocker? Would it help to move this info message up above the Display Title?
Not a blocker, in my opinion. (To move the message wouldn't help, though, as it's not valid for every title type.)
We could add something to the message text, but I'm not sure if it's helpful either. Something like this maybe:
Note: Any changes to the content below (and to the default block title) will affect all layouts where this reusable block is placed!
Anyways, to file a follow-up is a good idea!
...the brainstorming and wordsmithing for the text in #6531 gave me some inspiration. Here are some screenshots from my local (have not pushed them to the PR branch yet):
...expanded help text:
Thoughts?
@klonos - I like where this is all going. BUT, I'm not as excited as you are about "Shared custom content", but maybe I just need to let this idea settle in.
Also, I can imagine being hesitant to check the "Share this content across multiple layouts" box, because it's not clear to me which layouts it will be shared to? ;-)
Of course, this box does not actually share the content. It makes the content sharable. How do we express that in a succinct way?
Open to text suggestions. That's why I didn't push anything to the PR branch yet. Let me know if pushing that would help review and make suggestions better than the screenshots.
I have started working on the feedback from the code review in #4071, and this UX WTF came up (again) 😜😓 ... #2733 still has merit for all the other things mentioned there, but I am splitting this UX problem here, to its own issue:
I think that:
Bonus (while at it):
Notify the user that they may be about to edit something they weren't expecting.
.