Problem/Motivation
@xjm in #2449457: inconsistent checks in content_translation said this: "Also, the following remark mostly out of scope since this just follows the format of the existing documentation, but I think we should improve the way we document the additional keys on content_translation_entity_type_alter(). Ideally it'd be a scannable list in key: explanation form, like we used to have on hook_entity_info()."
Proposed resolution
Rewrite the documentation of this function.
Remaining tasks
Task | Novice task? | Contributor instructions | Complete? |
---|---|---|---|
Update the issue summary noting if allowed during the beta | Instructions | Done | |
Update the patch to incorporate feedback from reviews (include an interdiff) | Instructions | ||
Review patch to ensure that it fixes the issue, stays within scope, is properly documented, and follows coding standards | Instructions |
User interface changes
API changes
Beta phase evaluation
Issue category | Task, because Tasks are not functional bugs but represent things that 'need to be done'. |
---|---|
Issue priority | Normal because this is changing isolated documentation. This is more involved then a minor priority would be as a minor priority would just be fixing a typo or changing format. |
Unfrozen changes | Unfrozen because it only changes documentation. |
Comments
Comment #1
sasanikolic CreditAttribution: sasanikolic at MD Systems GmbH commentedComment #2
sasanikolic CreditAttribution: sasanikolic at MD Systems GmbH commentedComment #3
xjmSo as an example, I've just proposed the following documentation on that issue (based on @sasanikolic's patch):
By default, entity types that do not have a canonical link template cannot be enabled for translation. This can be overridden by setting the 'content_translation_ui_skip' key to true. When that key is set, the Content Translation module will not provide any UI for translating the entity type, and the entity type should implement its own UI. This is useful for (e.g.) entity types that are embedded into others for editing (which would not need a canonical link, but could still support translation).
In the updated docblock, there would be a preamble about the keys it adds to entity types, then a list of each key, formatted like so:
- content_translation_ui_skip: By default, entity types that do not have a canonical link template cannot be enabled for translation. Setting this key to TRUE overrides that. When that key is set, the Content Translation module will not provide any UI for translating the entity type, and the entity type should implement its own UI. This is useful for (e.g.) entity types that are embedded into others for editing (which would not need a canonical link, but could still support translation).
Comment #4
xjmComment #5
sasanikolic CreditAttribution: sasanikolic at MD Systems GmbH commentedChanged the description as suggested.
Comment #6
BerdirAnd now we need to do the same for the paragraph above that I think.
But not sure that works so well, because it's nested?
Comment #7
Patrick Storey CreditAttribution: Patrick Storey at Hook 42 commentedAdded remaining tasks template to issue summary.
Comment #8
Patrick Storey CreditAttribution: Patrick Storey at Hook 42 commentedAdding Beta phase evaluation template to issue summary.
Comment #9
jhodgdonThanks for the patch... but I think you've misunderstood the instructions in #3, and the list also needs to be formatted according to
https://www.drupal.org/node/1354#lists
So we need a "preamble" explaining what the list is as a whole, and then I think there are other keys besides content_translation_ui_skip to document?
Comment #10
phillamb168 CreditAttribution: phillamb168 as a volunteer commentedComment #11
phillamb168 CreditAttribution: phillamb168 as a volunteer commentedAdding a preamble and modifying the wording a bit. Based on what I've seen (cf core/modules/content_translation/src/ContentTranslationManager.php ~Line 69) this really has only one value available to override the link template check.
Comment #12
phillamb168 CreditAttribution: phillamb168 as a volunteer commentedHere's the interdiff
Comment #13
phillamb168 CreditAttribution: phillamb168 as a volunteer commentedComment #14
jhodgdon@philamb168 pinged me in IRC to ask about this issue. Record of conversation about what to do in this issue:
04:09:47 PM) jhodgdon: phillamb168: so... I'm taking a look at that whole doc block, as it is before this patch (which I didn't do before, sorry!)
(04:10:19 PM) jhodgdon: phillamb168: I think what xjm was meaning when she made the suggestion for this issue is this:
(04:10:53 PM) jhodgdon: phillamb168: if you start up at the top of that doc block, you'll see the 1st full paragraph says something like "blah blah blah there are a bunch of keys"
(04:11:12 PM) jhodgdon: phillamb168: or really that's at the beginning of the 2nd paragraph
(04:11:39 PM) jhodgdon: phillamb168: and then starting with the 3rd full paragraph, "Every entity type ...", the remaining stuff is defining a bunch of keys
(04:12:01 PM) jhodgdon: phillamb168: so the idea of this issue is that we should take all of the "here is a key" paragraphs and make them into a list.
(04:12:09 PM) jhodgdon: phillamb168: so it would start with something like:
(04:12:21 PM) jhodgdon: Here is a list of the keys you need to define for content translation:
(04:12:28 PM) jhodgdon: And then the first one would be:
(04:12:39 PM) jhodgdon: translation: Every entity type needs ....
(04:12:51 PM) jhodgdon: Then the next one would be:
(04:13:03 PM) jhodgdon: content_translation_ui_skip: By default, entity types ...
(04:13:26 PM) jhodgdon: phillamb168: actually each one of these would be a list item, see https://www.drupal.org/node/1354#lists
(04:14:05 PM) phillamb168: jhodgdon: so would it be OK to go ahead and rewrite that whole block?
(04:14:23 PM) jhodgdon: phillamb168: yes that is what this issue is really asking for I think, or at least we'll make it be asking for that. ;)
(04:14:42 PM) jhodgdon: xjm: that is what you were after right?
(04:14:46 PM) xjm: jhodgdon: yep!
And for reference, here's the start of the code/doc block that is referenced:
Comment #15
phillamb168 CreditAttribution: phillamb168 as a volunteer commentedOK, I've changed around some of the wording and added in the list formatting to the whole doc block.
Comment #16
phillamb168 CreditAttribution: phillamb168 as a volunteer commentedComment #17
jhodgdonThis looks great, thanks! The wording is very nice, and I've verified that the documentation has the same meaning as before. The patch covers all of the optional keys. Let's do it!
Comment #18
alexpottCommitted 40ba76c and pushed to 8.0.x. Thanks!
Thanks for adding the beta eval to the issue summary.