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.
Arianek and I decided to split up http://drupal.org/node/632716 so this is for the field module (it involves 6 files!).
The last patch can be found at http://drupal.org/node/632716#comment-2285200
Comment | File | Size | Author |
---|---|---|---|
#38 | number-hook-help-645776.patch | 1.15 KB | matason |
#34 | field-help-645776.patch | 9.31 KB | matason |
#32 | field-help-645776.patch | 9.23 KB | matason |
#30 | 645776fixes.patch | 10.54 KB | jhodgdon |
#26 | 645776reroll.patch | 10.54 KB | jhodgdon |
Comments
Comment #1
lisarex CreditAttribution: lisarex commentedadding tags.
Also, quick review (will reroll if time permits)... all module names should be capitalized!
Comment #2
arianek CreditAttribution: arianek commentedmanually cut this from the mondo patch
Comment #3
tobiasbComment #5
tobiasbmissing
</dl>
in field_helpComment #6
arianek CreditAttribution: arianek commentedi just added all appropriate caps and split the main field help from the submodules (2 patches now) because chx wasn't sure that they should have help at all and i'm thinking that the main one might be commitable earlier. the main field help is all very object oriented sounding, but i'm not sure what to do with it... needs input from @webchick.
Comment #7
arianek CreditAttribution: arianek commentedps. we don't have a link to the handbook page for field.module yet, but i didn't want to make a stub page on d.o in case cck is going to just be turned into that. also needs input from @webchick or maybe @jhodgdon on that one.
Comment #8
jhodgdonI think that all core modules should have at least an About section. You can enable these modules separately... why not make a help screen for them?
The current patch for Field itself needs capitalization updates in places.
Comment #9
jhodgdonCorrection: I guess you cannot enable or disable those field modules... So I'm wavering on whether they should have help text.
Anyway, here's a new patch for the Field module, with updated text.
Comment #10
arianek CreditAttribution: arianek commented- found one bad verb tense, and one caps - added these, and rolled a new patch for Field module.
- patch for all of the submodules in comment #6 still stands http://drupal.org/files/issues/help_field_submodules.patch
IMPORTANT: concerns still not addressed from above:
- the main field help is all very object oriented sounding, but i'm not sure what to do with it...
- we don't have a link to the handbook page for field.module yet, but i didn't want to make a stub page on d.o in case cck is going to just be turned into that (not sure how that works). if this is added, it should be the final sentence of the About paragraph, and read "For more information, see the online handbook entry for Field module." with "Field module" linking to the handbook page.
Comment #11
jhodgdonThe main field module cannot really be used by a user directly. It's a programmer's module, really. So I think the tone is OK... but thenagain I'm a programmer...
I do NOT think we should turn CCK pages into Field pages. Creating a new one would be better. It doesn't work exactly the same, and the API is quite different even if the UI is similar.
Comment #12
jhodgdonOK. I changed this around, I think you'll like the new version's tone better.
Again, this patch is just for the Field main module; sub-modules are in http://drupal.org/files/issues/help_field_submodules.patch on #6 above.
Comment #13
arianek CreditAttribution: arianek commentedyay, love it. found one little code mistake (missing * in comments, prolly old) and a typo, otherwise lovely. and also added a link to new stub page in the handbook.
reposting both the field, and separate field submodules patch (if @webchick sees fit to commit it) for RTBC
Comment #14
webchickLike Update status module, this one is a bit more technical in its wording than others. But as jhodgdon points out, it is at its heart a programmer module, so this tone is probably fine in this case. I went ahead and committed help_field9.patch to HEAD.
The submodules patch though, I'm a less enthused about. We already conveniently itemize the list of fields in the field module help text and I think that's much better from a usability standpoint than clicking on 50 different links. OTOH, when you download date module in contrib, you're going to get a help page for it, and then it'll be weird that you don't get one for these others. Hrm.
But I also don't see the point in committing the patch as-is when it has just a single line that repeats what was already in the Field module help. :P
Field SQL storage module too is a tricky one. That one is *completely* in the dev space and nothing user-facing at all. Since the help here is geared primarily toward site builders, I kind of lean toward not providing any help since it'd do more harm than good, probably. But... not sure.
So I'm setting the issue back to active so we can discuss the sub-modules more. I don't feel like we've quite reached a consensus yet.
Comment #15
jhodgdonA few thoughts:
- Apparently, Drupal is no longer even showing the required modules on the Modules page (WTF? Who came up with that idea?). Modules field, list, number, field sql, etc are all marked required. So an admin will not really be aware they have them turned on, and has no way to disable them. I think?
- I can't think of anything to say for the list, number, and text sub-modules beyond the one-liners that are already there (and duplicated in the field module). Another alternative would be to put them into a paragraph/DD in Field, and link to the help files for the relevant modules. Something like this:
- If someone comes up with some other field storage modules, then I think we would want help for SQL Storage in order to explain what it does specifically, since (one would hope) the other field storage modules would also have help to explain their benefits. Maybe we should mention field storage in the main Field help and link to SQL storage module as one alternative, to set the context? Or maybe not. Do we think other field storage mechanisms will be forthcoming?
Comment #16
jhodgdonOK, here's a proposed patch for Field and its submodules. Basically moving all the information into the submodule pages, and linking to submodules from the main Field help.
Comment #17
jhodgdonAs a note, the Field Storage SQL module already had a hook_help() implementation in it, so if we decide to leave Field as it is an want to not have help for Storage SQL, we would need a patch to remove that function.
Comment #18
webchickOh, now that's interesting. What if we took this one step further and looped through all of the available field types in the system (module_invoke_all('field_info')) and printed them out in a bulleted list, with links to their help pages if they exist? Then the Field help page could serve as a really useful reference point for all available field types in the system.
The decision to hide required modules from the modules page was made a long time ago by the UX team. I think the idea was that there's nothing you can do with those choices, so why bother eating up space with them? I agree though that it's sub-optimal here. It might be worth revisiting that discussion to see if we want to change this now that we have permissions/help links from the modules page.
Comment #19
jhodgdonYes, I think we should revisit that discussion.
Regarding a bulleted list, that is not a bad idea, but wouldn't that give us a list of fields rather than a list of modules that contain fields? I guess we could do a foreach( module_implements() ) or something like that, though.
Let me see what I can do with that...
Comment #20
jhodgdonOK, here's a new patch, and a new screen shot for Field (sub-module screen shots are same as above). This was a good exercise -- pointed out that Taxonomy is another core Field module. :)
Note that due to #563390: Seven theme lacks formatting on common html elements such as lists, paragraphs & <code> the bullet list formatting is not working in the screen shot. Sigh.
Also, I decided that the list should include widget modules as well as field modules. Hope that's OK.
Comment #21
arianek CreditAttribution: arianek commentedI really like the direction is going on this, for what it's worth. :-)
But I'll also add my WTF to hiding the core required modules. There are so many reasons I think this is a bad idea, I won't even start listing them here... but seriously, would suggest that needs to be reevaluated!
Comment #22
jhodgdonThis is the issue:
#293223: Roll back "Hide required core modules"
I have just reopened it.
Comment #25
jhodgdonI was afraid that patch wouldn't fly. Here's a reroll.
Comment #26
jhodgdonMinor typo there, try this one.
Comment #27
arianek CreditAttribution: arianek commented- not a fan of the "but" in field help - maybe end the sentence or a semi-colon? "The Field module provides the infrastructure for fields and field attachment, but the field types and input widgets themselves are provided by additional modules."
- "drupal.org" should be "Drupal.org"
- still don't have this code wrapping all sorted, but i think this line is too long (over 80) "+ // Make a list of all widget and field modules currently enabled, in order"
otherwise thumbs up
Comment #28
arianek CreditAttribution: arianek commentederrr..... one more thing, the first field type in the generated list that has been added to the field help page "file" isn't displaying as a link
Comment #29
jhodgdon#28: It will only display as a link once the File help is committed. The links are to the help pages, and if the help page doesn't exist, it displays without a link.
#27: Really, we are saying "Drupal.org"? Code wrapping - correct, need to fix that. "but" -> ";" agreed.
Comment #30
jhodgdonRE #29, yes you are right, I guess we are saying Drupal.org. Live and learn.
Anyway, here's a patch that fixes two instances of but (replace with ;) and the code wrapping and the Drupal.org fix.
By the way, all of those except the code line wrapping were in the previusly committed version of the module help. :)
Comment #31
arianek CreditAttribution: arianek commentedthat doesn't surprise me - fresh eyes catch new things now that we're not in full on crunch mode. ;-)
ya i had to look up the Drupal.org a while back, but that be the rule! being that the link will work once committed, i think that covers it all - looks good good RTBC
Comment #32
matason CreditAttribution: matason commentedUsed theme_item_list instead of manual construction of ul list of field types.
Comment #34
matason CreditAttribution: matason commentedI ran cvs diff in the Drupal root directory this time, hopefully that will sort the fail.
I've re-worked the way the enabled field and input widget modules list is constructed, I found array_merge() left me with duplicate values in the array (because of numeric keys), I used array_unique() to sort that out, is there a more elegant approach?
I also used sort() to sort the array on values.
Comment #35
matason CreditAttribution: matason commentedSetting to "needs review".
Comment #36
webchickI went to the testing bot page and it showed green, tho it hasn't been reflected yet here.
Thanks a lot for cleaning up the implementation there, matason!
This looks great. Committed to HEAD.
Comment #37
jhodgdonThe patch that was committed didn't include the help screen for the Number sub-module for some reason (which was in #30).
Comment #38
matason CreditAttribution: matason commentedGood catch, attached patch has hook_help() for the number module.
Comment #39
jhodgdonLooks good to me, works, etc. Let's get it in.
Comment #40
webchickCommitted to HEAD!