UMN Usability: Move 'Content Types' to Site Building

matt2000 - March 30, 2008 - 06:48
Project:Drupal
Version:7.x-dev
Component:node system
Category:bug report
Priority:critical
Assigned:matt2000
Status:closed
Description

If I remember correctly from the Usuability panel at DrupalCon Boston, it was 100% of users who thought 'Content Types' should be under Site Building. I strongly agree.

These patches change all references to 'admin/content/types' to 'admin/build/types'.

This is my first ever patch submission for any software project. I've probably broken all kinds of standards & ettiquette, so please be patient and let me know the bets way to do it in the future. I figured I'd learn by doing. :-)

path.inc_.patch is for the file in /includes. Everything else should be obvious enough.

AttachmentSize
comment.module.patch1.03 KB
content_types.inc_.patch660 bytes
locale.install.patch804 bytes
node.module.patch2.07 KB
path.inc_.patch170 bytes
system.admin_.inc_.patch168 bytes
translation.module.patch1020 bytes

#1

matt2000 - March 30, 2008 - 06:57

oops. Did I make my patch files wrong? Maybe these are better...

AttachmentSize
translation.module.patch2.28 KB
system.admin_.inc_.patch581 bytes
path.inc_.patch455 bytes
locale.install.patch1.24 KB
comment.module.patch2.02 KB
content_types.inc_.patch1.07 KB
node.module.patch4.7 KB

#2

catch - March 30, 2008 - 11:44
Category:feature request» task
Priority:minor» critical

OK all the individual patches look fine. The only thing is that they're individual patches instead of one big one.

Depending on what you're using, you should be able to do cvs diff -up > yourpatch.patch from the root of the drupal directory - then you get one big patch file with all the changes to each file. If you use Tortoise CVS or similar, it'll also roll patch files for all changes from the root.

This makes it a lot easier to read and test, since you only have to look through one thing (and only have to apply one patch to test it). http://drupal.org/patch/create has everything you need. If you checked out Drupal 7 via cvs, then you may just be able to re-roll with your existing changes.

Massive +1 from me. Thanks!

#3

Dries - March 30, 2008 - 21:33

I think this is an important change. We should also revisit the terminology as "content type" was highly confusing, but that could be left for another patch.

#4

matt2000 - March 31, 2008 - 02:04

OK, here it is as one patch file. Thanks for the lesson.

As for the terminology, 'Content Types' seems natural to me, and I can't think of anything better; but then, I've been working with Drupal for a year. Did the test participants have any suggestions?

The term that's been difficult for my clients is 'page.' To most people, every refresh of their browser generates a "page." The theme is the look of the page. They might want a 'page' that lists their articles (which is usually a View).

I often create a generic content type called 'post,' and to them, a 'page' is made up of one post, or a list of posts. They don't understand taxonomy listings as a 'page (generically) of pages (the drupal content type).'

Finally, I noticed that the term 'taxonomy' has returned to replace 'categories' in the admin menu. Was this an intentional decision?

AttachmentSize
build-types.patch14.16 KB

#5

catch - March 31, 2008 - 10:41

Patch looks lovely, thanks for re-rolling. I'll try to test it a bit later today.

I don't think there were any suggestions about terminology from participants (we didn't speak to any of them directly ourselves) - but 'page' 'story' 'content' 'content type' 'create content' and 'post' caused a lot of problems. Story has been changed to 'article' in D7, I started a discussion in the usability group about the general issue of what to call everything else.

However like Dries suggests, we should try to move the menu item asap, and open another issue for any terminology changes.

Taxonomy change from categories is here.

#6

flobruit - March 31, 2008 - 20:54
Status:patch (code needs review)» patch (reviewed & tested by the community)

Patch applies cleanly and replaces all instances of admin/content/types. Menu item is moved after cache is cleared.

#7

keith.smith - March 31, 2008 - 21:13

While reviewing this patch, I notice that there is an instance of "Story" that should have been changed to "Article". I've created a patch for that at #241021: Reference to "Story" in help text not renamed., rather than rolling it into this one. I can reroll #241021: Reference to "Story" in help text not renamed. after this gets committed.

#8

Dries - April 1, 2008 - 20:03
Status:patch (reviewed & tested by the community)» patch (code needs work)

I applied this patch but it fails at one chunk. It will need to get rerolled.

#9

keith.smith - April 1, 2008 - 20:16
Status:patch (code needs work)» patch (code needs review)

Rerolled. The hunk failed due to the renaming of that one last instance of "Story" to "Article".

AttachmentSize
build-types-1.patch14.18 KB

#10

flobruit - April 1, 2008 - 20:26
Status:patch (code needs review)» patch (reviewed & tested by the community)

Re-rolled patch to adapt to the change from #241021: Reference to "Story" in help text not renamed.. Just changed Story to Article in the patch directly and it applies cleanly. No other changes, so marking as RTBC.

AttachmentSize
build-types-241021-9.patch14.09 KB

#11

Dries - April 2, 2008 - 20:45
Status:patch (reviewed & tested by the community)» patch (code needs work)

I think a recent patch might have broken this patch again. Sorry.

#12

flobruit - April 2, 2008 - 21:18
Status:patch (code needs work)» patch (reviewed & tested by the community)

Re-tested the patch on a current checkout from HEAD, and it works fine (applies cleanly with both the patch command and Eclipse).

#13

catch - April 7, 2008 - 13:57
Status:patch (reviewed & tested by the community)» fixed

http://drupal.org/cvs?commit=109329

yay!

#14

Anonymous (not verified) - April 21, 2008 - 14:02
Status:fixed» closed

Automatically closed -- issue fixed for two weeks with no activity.

#15

recidive - May 10, 2008 - 08:17
Status:closed» patch (code needs review)

The applied patch was incomplete. Editing and deleting content types are still on 'admin/content'.

Patch to correct this attached.

AttachmentSize
content_types_remaining.patch2.89 KB

#16

Dries - May 10, 2008 - 13:23
Status:patch (code needs review)» fixed

Good catch. Committed to CVS HEAD. Thanks.

#17

webchick - May 21, 2008 - 02:26
Category:task» bug report
Status:fixed» active

This patch broke translation.test (at least). Please fix.

#18

catch - May 21, 2008 - 08:56
Status:active» patch (code needs review)

translation.test was reviewed on April 10th, after the original patch here was committed. So the test should really have been failing due to the bad path in the code and passed when the followup patch went in rather than vice versa.

I can't find admin/content/node-type anywhere else in core, so hopefully this one will fix it.

AttachmentSize
build.patch1.05 KB

#19

catch - May 21, 2008 - 10:20
Status:patch (code needs review)» patch (reviewed & tested by the community)

Since the patch is trivial, and all translation tests pass with it applied, I'm going to mark my own patch RTBC.

#20

catch - May 21, 2008 - 11:00
Status:patch (reviewed & tested by the community)» closed

Marking back to closed in favour of: http://drupal.org/node/260499#comment-851924

 
 

Drupal is a registered trademark of Dries Buytaert.