Closed (fixed)
Project:
Drupal core
Version:
6.x-dev
Component:
documentation
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
27 Nov 2006 at 15:26 UTC
Updated:
2 Jan 2008 at 10:42 UTC
Jump to comment: Most recent file
Comments
Comment #1
neclimdulI'll take a shot at it.
The listed complaints are taken care of and I tried making the content more readable and better organized as well.
Comment #2
dries commented"Forum topics can also have their own URLs like other content types."
Small remark: a forum topic is not a 'content type'. I think you meant 'content'.
Otherwise looks OK.
Comment #3
webchick"content type" would be consistent with the menu item administer >> content >> content types, but why don't we just remove that sentence altogether because it doesn't really add anything. However, you always want 3 items in a list, so I'm not sure what that other item should be.
How about something like "You may lock a thread by turning the commenting option to disabled on the parent thread." I've seen that come up on the forums every now and then, and it more closely matches the rest of the items there, as it's talking about a specific administrative action you can do.
Comment #4
neclimdulI like webchick's third item and have modified the patch to include that instead of the URL bit. That's really path help anyways and might confuse people if the path module isn't turned on.
Comment #5
drummpatching file modules/forum/forum.module
Hunk #1 FAILED at 1.
Hunk #3 FAILED at 28.
2 out of 3 hunks FAILED -- saving rejects to file modules/forum/forum.module.rej
Comment #6
webchickWe're in a string freeze ... this can't get applied until 6.x
Comment #7
webchickMoving to documentation component.
Comment #8
sutharsan commentedPatch re-rolled against current 6.x-dev.
Comment #9
keith.smith commentedThis mostly looks ok, except the first paragraph is a little clunky, though less so after this patch. I don't have any concrete suggestions about it right this moment though.
In the second paragraph, the text:
could probably just be
Comment #10
keith.smith commentedwebchick hit the major point here in when she created the issue:
This patch does remove PHPBB, and takes out the taxonomy/comments stuff that is already handled if forum has been enabled. However, I also fiddled with most (or all) of the texts. Forum is a strange one, because it has several tough concepts that all blend together. Containers containing forums with forum topics that have comments (and replies).
Patch attached for review.
Comment #11
keith.smith commentedComment #12
keith.smith commentedOK. So not so much. I'll attach a patch when the issue directory comes back online.
Comment #13
keith.smith commentedAnd, here we go. This patch goes with the comments in #10.
Comment #14
catchLooks pretty good to me. Only thing I'd say is I didn't notice a mention of drag and drop?
Comment #15
keith.smith commentedsigh. yeah, I forgot about that.
Comment #16
keith.smith commentedOk. To the admin/content/forum page, this adds:
To rearrange forums and containers, grab a drag-and-drop handle under the Name column and drag the forum or container to its new location. (Grab a handle by clicking and holding the mouse while hovering over a handle icon.)
Is this sufficient for drag-and-drop? If so, I'll use this as a model on....uh...the block, menu, taxonomy and profile pages.
Note:
Comment #17
michelleSince I don't have Drupal 6, I only read through the patch file. This also means I can't comment on the accuracy of text related to drag and drop or other content types.
In general, I thought the text was much improved. I only had two small nitpicks:
"Description of or guidelines for" - This may just be me but that wording is jarring.
"selecting Disabled under Comment settings while editing a forum topic will unpublish all existing comments on the thread, and prevent new ones." - This isn't true in 5.x. I don't know if it is in 6.x.
If someone with 6.x could check whether comments are unpublished to verify that part, I think this is RTBC.
Michelle
Comment #18
keith.smith commentedYes, I used the wrong term there.
Comment #19
keith.smith commentedOk, addressed both of these issues:
* Disabled hides comments, rather than unpublishes them
* Changed the Description of or guidelines for to Description and guidelines for
Comment #20
keith.smith commentedmeh.
Comment #21
keith.smith commentedAdded Remember that your changes will not be saved until you click the Submit button at the bottom of the page. on the drag-and-drop description.
Note that Submit should perhaps be Save and that Reset to Alphabetical should be Reset to alphabetical (and should work) but that's another issue.
Comment #22
gábor hojtsy- IMHO there is information loss with replacing "delete all sub-forums and associated posts as well" with "also delete all of its contents". Even if the former was not exactly nice, it gives us more info (posts *will* be deleted). This is not clear in the new version. There are multiple instances of this in the patch.
- "Minimum number of comments necessary to be considered "hot"." What is considered? I am not sure the field title gives enough context.
- We avoid glueing stuff together like "@createtopic", instead we do "@create-topic" or something along those lines, maybe even @create.
- "contained within a container" :)
- the list items have ; at the end while other similar lists eg. in block module have dots if I remember right(?)
- Let's name the button "Save forums" or "Save" (I know there is an ongoing debate about this) with this patch, so the help text will also be right at the same time!
- I like the 'drag handler under named column' type of way of talking about the drag and drop which is compatible with RTL, although if an RTL theme is used, hopefully it will be accompanied with an appropriate translation, so it will be translated to however it is positioned. :) Anyway, this looks like a good standard even thinking about possible form_alter()s, which might modify how the form looks.
- @forum_vocab same as with @createtopic, use @vocabulary here or something like this.
Generally, these are not huge showstoppers, but it would be great, to get these right, and finally remove the phpBB reference, so keep up the great work :)
Comment #23
keith.smith commentedGreat, thanks for the review. I'll see about getting this fixed up ASAP.
I had thought -- much later, after I had already posted -- that obviously, RTL translators would likely change left to right in their translations. So I'm not certain that's as big a deal as I thought.
Slightly off topic, as an update:
I know the string freeze is imminent: as of the weekend, we were lacking major help text rewrites for: node, block, system, menu and locale. Drag and drop needed to be added to forum, and profile. There are also some less pressing issue for contact, comment, filter and (one part of) taxonomy.
Locale and block are in, and most of the rest of the major ones need a review, or are, like this one, recovering from a review. :)
The only major one that I am aware of that where there's not a good patch at or near some stage of completion is system, and man, does that text needs help. I'm going to get something in the queue for it today though, unless someone else beats me to it.)
Comment #24
keith.smith commentedI have the feeling I forgot something here, as I am trying to hurriedly put this back into review (am late for dinner).
- the delete confirmation messages were less informative, so I put them back to near what they were.
- same for the hot description "hot" (considered was in there originally)
- fixed "@createtopic" and the legacy instance of "@forum_vocab"
- This should no longer contain "contained within a container". :)
- added periods to the ends of list items.
- named the button "Save" rather than "Save forums" since it also appears on taxonomy forms. Fixed the reference to it in the help. I did a bit of basic testing and the rename does not appear to have broken anything.
I think that was everything...
Comment #25
gábor hojtsyI looked through this one, and all my comments were acted on, so it looked fine. Even the explanation of what is removed when a forum gets deleted was expanded on, even though you did not mention it in your list. Thanks, committed.
Comment #26
(not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.