Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
The links on the administration pages of the Custom Block module, the Custom block library, do not fit with the page names. Those texts and links needs to be updated so that they are correct.
Proposed resolution
Update the UI texts Custom Block administration pages, using the wording from the hook_help text.
Remaining tasks
Update the UI texts.
User interface changes
This is a UI text change.
API changes
None.
Data model changes
None.
Comment | File | Size | Author |
---|---|---|---|
#69 | 2572545-68b-interdiff.txt | 1.7 KB | lokapujya |
#68 | 2572545-68-interdiff.txt | 1.77 KB | lokapujya |
#68 | 2572545-68.patch | 6.87 KB | lokapujya |
#66 | update_ui_texts_custom_block-2572545-63.patch | 6.87 KB | Manjit.Singh |
#61 | interdiff-2572545-51-61.txt | 2.47 KB | pguillard |
Comments
Comment #2
ifrikComment #3
DuaelFrComment #4
ehegedus CreditAttribution: ehegedus commentedOn it
Comment #5
ehegedus CreditAttribution: ehegedus commentedNot clear how much change should I make, maybe I'm just overthinking it, some detailed feedback would be appreciated. Leaving unassigned, maybe someone with more documentation experience solves it easily.
Comment #6
mikebell_ CreditAttribution: mikebell_ as a volunteer commentedI took a look at this and have a few comments:
1. The help text doesn't reference the fact it's custom blocks, not sure if this is relevant but if the titles use the name then maybe the help text should also reference this.
2. http://drupal8.dev/admin/structure/block/block-content -> has the title "Custom block library" the url doesn't reference a library or custom. Maybe this should be another issue all together.
3. http://drupal8.dev/admin/structure/block/block-content/types -> similar to above, block-content doesn't make sense considering it's a library.
Comment #7
jhodgdonApparently this needs to be "rc deadline" because it changes translatable UI text strings. See https://groups.drupal.org/node/484788
Comment #8
dpopdan CreditAttribution: dpopdan commentedComment #9
jhodgdonThe release candidate came out yesterday, so the deadline was missed. So... see https://www.drupal.org/core/d8-allowed-changes ... under Evaluating, item 4:
So unless we really need to ask for special committer discretion here, this needs to be postponed to 8.1.x. In which case we cannot work on it for a few months and so I am going to at least for the moment un-assign it. Thanks for your interest though!
Comment #10
no_angel CreditAttribution: no_angel as a volunteer commented- Barcelona2015, rc deadline
Comment #11
pguillard CreditAttribution: pguillard commentedWhat do you think of these texts ?
For the "Blocks" tab :
For the "Types" tab :
I have not taken #6 into account, because it seems that the title is for the entire section
Comment #12
jmarkel CreditAttribution: jmarkel as a volunteer and commentedThis is may be a more general issue across Drupal but "Add" and "Create" are being used interchangeably here although they are not synonyms. I.e. the "Add custom block" and "Add custom block type" buttons and their target forms/pages aren't used to add blocks (which implies that they could be being imported from elsewhere) but instead to create new blocks/block types.
I mention this because while the changed page text in the #11 patch is fine as far as it goes, the use of the word "create" ("allows you to create...") there and in the hook_help() text is inconsistent with the button actions and page titles, which use the word "Add."
I think it would be best to make the action verb 'create' rather than 'add' but, if left at 'add' the help/description text should be consistent (IMO).
Comment #13
jhodgdonI have no strong opinion on "create" vs. "add" except that we should probably use the same terminology in the Help as we do in the UI, so if the button/link says "add", the help should also say "add", right?
Regarding #11, I like the Types text, but not the Blocks text quite as much... I found it to be confusing. It seems to be referring to itself rather than pointing out to go to the Types page to define types?
Screen shots would also be helpful for the next review, so we can see the tabs/help/buttons/links in context without having to start up our own instance of simplytest.me etc. Thanks!
Comment #14
pguillard CreditAttribution: pguillard commentedThis is what I did :
What do you think ?
Comment #15
no_angel CreditAttribution: no_angel as a volunteer commentedIs 'library' used here because it is referencing created 'custom blocks'? Why not use 'list' instead or just 'Custom blocks'?
Just a question: Isn't this just another 'type' that can be created with fields like comments, content, ... and once create a list of comments, content is viewable.
Comment #16
snehi CreditAttribution: snehi as a volunteer and at Publicis Sapient for Publicis Sapient commentedWhat about this ?
Comment #17
no_angel CreditAttribution: no_angel as a volunteer commentedComment #18
jhodgdonGood! I like the fact that now the help text actually matches the UI.
I had one moment of "huh, what??" when I read the help text though, due to punctuation:
So what this is trying to say is that you can (a) add, edit, delete blocks here and (b) if you want to define a custom block type, you need to go to the Block types page. But due to the punctuation, when I read the end of this sentence, my first thought was that I had to go to the Block types page in order to add/edit/delete blocks.
I think it would be clearer if this was made into two sentences, more like the help you have in the patch for the Types page:
And as a note... The issue summary mentions the hook_help text, so we probably also need to look at the text on:
- The main Block and Custom Block help pages (admin/help and click on Block or Custom Block)
- The Block layout page help text at the top
and make sure it is all consistent.
Also the issue summary suggests changing the UI text and not the help... I don't care too much which we do as long as they are consistent, but the issue summary needs an update if we're doing it this way.
Comment #19
miteshmapsplitted text into two sentences.
Comment #20
jhodgdonYes, this reads much better. Thanks!
So, I went back to the issue summary, and also started up simplytest.me to look at what the help and pages look like for a site, with this patch. There are still many inconsistencies between the UI text and the help text:
Links:
- Custom block types => admin/structure/block/block-content/types
- Custom block library => admin/structure/block/block-content
- Block layout page => admin/structure/block
- The page title of this page is "Custom block library", which is also the tab title.
- The sub-tab navigation title is simply "Blocks".
- The button says "Add custom block"
- The type column is called "Block type"
- The help at the top of the page says:
Links:
- Block types page => admin/structure/block/block-content=
- The page title is still "Custom block library", which is also the tab title.
- The sub tab navigation title is simply "Types".
- The button says "Add custom block type"
- The types column is called "Block type"
- The help at the top of the page says:
Links:
- Blocks page => admin/structure/block/block-content
- The page title here is "Block layout".
Can you spot all the inconsistencies? There are a lot... Here is a list of what I think needs to be updated... at least these are my suggestions:
Thoughts?
Comment #21
no_angel CreditAttribution: no_angel as a volunteer commentedComment #22
mikemiles86Comment #23
HezzieB CreditAttribution: HezzieB as a volunteer commentedPeezy & I are working on this at the Boston Sprint Weekend.
Comment #24
HezzieB CreditAttribution: HezzieB as a volunteer commented@jhodgdon What do you think about changing the button from "Save block" to "Save and Place Block" for your issue (#5) below:
Comment #25
peezy CreditAttribution: peezy as a volunteer commentedAdding a note that @hosttor is helping as well.
Comment #26
hosttor CreditAttribution: hosttor at Hosttor commentedThis addresses number 3 above
Comment #27
peezy CreditAttribution: peezy as a volunteer commentedThis patch is for 1, 2, 4, and 6.
Comment #28
peezy CreditAttribution: peezy as a volunteer commented...and both of the patches together.
Comment #29
peezy CreditAttribution: peezy as a volunteer commentedComment #31
hosttor CreditAttribution: hosttor at Hosttor commentedComment #32
hosttor CreditAttribution: hosttor at Hosttor commentedComment #33
hosttor CreditAttribution: hosttor at Hosttor commentedComment #34
Saphyel CreditAttribution: Saphyel commentedIt works as #28 said
Comment #35
beanworks CreditAttribution: beanworks as a volunteer commentedPatch (update_ui_texts_customs_block-2572545-28) applied to Acquia desktop dev (dry run):
Problems completing. Is this supposed to be preceded by other patches?
Comment #36
Saphyel CreditAttribution: Saphyel commentedComment #37
lokapujyabeanworks, what do you mean by "problems completing"? As you can see, the patch applied and passed testing in #28. Oh, just saw the attached image. Are you applying to 8.1.x?
Comment #38
jhodgdonThanks for the patches and comments!
So...
Regarding comment #24, I don't think that changing the save button on the configure blocks page to say "Save and place block" is relevant to this particular issue at all. If you want to make that suggestion for Drupal Core, please make a separate issue. we try to keep issues focused on one thing, and this one is about the "custom block" functionality, not about the generic block configure and place functionality. And this change would not help to address item #5 in comment #20, which is also about the custom block functionality and not about the configure block forms.
Regarding the patch in #28, I do not believe that it fixes the problems identified in #20. To whomever creates the next patch, I will suggest that you read #20 carefully, and then before submitting your patch, you should go to the 4 pages that are mentioned in #20 and make sure that all references on those pages are updated to the 6 points mentioned in #20. They aren't in this patch.
Thanks!
Comment #39
no_angel CreditAttribution: no_angel as a volunteer commentedAssigning to me. Will look at patch in #19 and comments in #20.
Comment #40
pguillard CreditAttribution: pguillard commentedI guess this patch matches the 6 points mentioned in #20
Concerning point #5, I suggested :
Comment #41
jhodgdonI think this all looks very good now, thanks! The top-of-page help, page names, table headers, and tab names are all consistent with each other within the Custom Blocks module UI now.
So, let's just fix a couple of minor grammatical/wording/accuracy issues introduced by the patch (or pre-existing):
a)
- The word "them" at the end of this sentence is ambiguous: it could be referring to blocks or block types.
- I think "a Blocks page" should be "the Blocks page"
- Saying that the Blocks page just "lists" the blocks is kind of misleading. It's a page you can use to manage them.
So I think this could be worded better. Maybe something like:
... allows you to create and manage custom block types and content-containing blocks from the Custom block library.
This would link to the Blocks page within the library, but the Block types page is linked there, so I think it would get across the point that you can manage both types and blocks?
b)
The wording here is weird. It starts with "Users" and then says "you have defined". Probably would be better to say something like
... of each defined custom block type, ...
c)
There is no "blocks" or "Place blocks" list on the Block layout page. There is only a list that appears if you click on "Place block". So these seem kind of misleading.
I think these two areas would be better if they did something more like what is in this help:
(maybe with the addition of "you can" or something like that.
Comment #42
mahaveer003 CreditAttribution: mahaveer003 as a volunteer and at Valuebound commentedComment #43
mahaveer003 CreditAttribution: mahaveer003 as a volunteer and at Valuebound commentedAdded the patch with above changes.
Interdiff added.
Comment #44
mahaveer003 CreditAttribution: mahaveer003 as a volunteer and at Valuebound commentedComment #45
mahaveer003 CreditAttribution: mahaveer003 as a volunteer and at Valuebound commentedComment #46
snehi CreditAttribution: snehi as a volunteer and at Publicis Sapient for Publicis Sapient commenteddon't you think it needs quotes. i may be wrong.
Comment #47
jhodgdonMy feedback in #41 (a) was not addressed in this patch, or at least the interdiff doesn't show that it was fixed, as far as I can see.. (c) is not either, at least not completely.
Please read the reviews and if you don't understand what is being asked, rather than making a patch, ask a question. Thanks!
Comment #48
harsha012 CreditAttribution: harsha012 as a volunteer and at Red Crackle commentedAdded the patch as per the comment #41.
Comment #50
pguillard CreditAttribution: pguillard commentedComment #51
pguillard CreditAttribution: pguillard commentedSo.. I started from my previous patch at #40 and applied suggestions from #41.
@harsha012 : I guess you get wrong code with a copy paste, I found bad Urls and some "em" tags disappeared. Then I found out some code was missing from my previous patch, I guess you had things both in git working and staging areas and your git diff didn't catch everything.
@jhodgdon : I do not answer your interrogations, because I lack judgment :-), but mostly because I tried to strictly apply suggestions from #41 and put this patch back on track.
Comment #52
jhodgdonYES, thanks. This all looks much better to me! Assuming the bot agrees...
Comment #53
jhodgdonChanging component here. This is not really "documentation".
Comment #54
catchWhy is it 'Custom block library' in the link text, but 'custom block library' when not linked. I'd expect 'custom block' all the time since we don't treat 'block' by itself as a proper noun.
Comment #55
snehi CreditAttribution: snehi as a volunteer and at Publicis Sapient for Publicis Sapient commentedDone. Please review.
Comment #57
jhodgdonNormally, in UI text such as help, if we make a link to a particular page, we use the exact page name as shown in the UI when you go to the page. The page is called "Custom block library", so I think when linking to that page it should be capitalized.
However it should probably say
Comment #58
Manjit.SinghIs that the reason that automated test fails ?
Comment #59
pguillard CreditAttribution: pguillard commentedShould I rename patch #51 to #59 and set "Needs review" again ?
Comment #60
jhodgdonCan we add the word "page" in there, possibly in several places, as noted in #57?
Comment #61
pguillard CreditAttribution: pguillard commentedSure.
(I started from #51 to add "page")
Comment #62
jhodgdonLooks good. However, there are now two lines in this patch where in the help, "Custom block library" is capitalized instead of being just "custom block library" even though it is not a link.
here's one
Here's the other.
Comment #63
alvar0hurtad0Comment #64
alvar0hurtad0Here, are #62 changes over #61 patch.
Comment #65
alvar0hurtad0Please, ignore preovious patch, I've uncommited changes from other issues and I'm mixed them (so sorry).
This is the patch.
Comment #66
Manjit.Singhchanged
Custom block library
tocustom block library
where there is no link. Please verify it once.Comment #67
jhodgdonThis still says "Custom block library". So does another page. So that last patch is not good. I didn't look at the other two. Please why are you all adding multiple patches to this issue? One at a time!
Comment #68
lokapujyalets see if this interdiff works:
git diff --color-words
Comment #69
lokapujyaslightly better maybe.
Comment #71
jhodgdon--color-words is only good for looking at things in your terminal. Please don't use it for making interdiff files, as they're not readable online. You need to just make regular interdiff files.
Anyway, the latest patch looks good, thanks!
Committers: I am not sure what the string change deadline is for the Beta cycle, but if that hasn't passed, I think it would be good to consider this for 8.1.x as well as 8.2.x.
Comment #74
catchCommitted/pushed to 8.2.x and cherry-picked to 8.1.x. Thanks!