Posted by robertDouglass on June 17, 2006 at 10:22am
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | node.module |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | Needs text review, Needs usability review, Screenshot, Usability |
Issue Summary
It is irritating that one cannot see what type of node one is editing. I would like to have the node editing form always display the node type somewhere on the page. This could be a drupal_set_message, or help text, or anything else... just as long as it appears on the form.
If we can come to a consensus on the most user friendly way to do this, I'll roll a patch.
Comments
#1
This patch makes the node submission and node edit forms consistent. They now both say either "Submit Foo" or "Edit Foo" through drupal_set_title. This is an important usability improvement, especially now that people will be creating many different but identical node types. It is otherwise impossible to tell by looking at a node edit form what type of node is being edited.
#2
#3
Patch applies cleanly, does something useful, and the coding style looks good to me. It's also nice and small, which is even better. :-)
#4
Changing the title of the post is a bad idea and breaks with the purpose of having tabs. Won't fix the current solution.
#5
Dries, for me, this is a display issue. I don't understand what you mean about changing the title of the post.... I'm not intending to do that and didn't think that my code did that. The comment about tabs I don't understand.
For me, on a node edit form, I want to see what type of node it is. It is completely a display issue. Right now, it is possible to end up looking at any number of node-edit forms, each with a Title and a Body, and not know what type of node you're editing, which seems totally wrong. Note that in the current solution, the first time the node form is called, drupal_set_title is used to display what type of node you are creating. I moved this call up higher in the processing and added logic to decide whether you are submitting or editing an existing, and changed the wording of the title that gets set. I didn't notice that this screwed up tabs or changed the title of the post.
Please explain.
#6
Does this still applies to cvs?
robertDouglass: Any consensus about this patch? Or Won't fix the current solution is final?
Moving to cvs.
#7
The issue still remains. It is a real usability issue. The patch won't be accepted so it needs a new one.
#8
Moving all usability issues to Drupal - component usability.
#9
#10
I agree this would be a useful addition. The title of the node however should remain consistent. The simple solution would be display the node type as markup displayed on the form itself.
#11
This would be a nice improvement. I know I have resaved unchanged nodes to see what type it was. ("your Foo has been saved")
I'd like to see a screengrab of what the patch does, it seems we're still a bit confused about Dries's comment.
#13
#14
New version of the patch which adds the node type as a form element
#15
The last submitted patch failed testing.
#16
Re rolled from root of drupal
#17
The last submitted patch failed testing.
#18
Attached wrong version of patch above.
#19
The last submitted patch failed testing.
#20
New version of the patch which corrects a few other issues in that node type is only displayed when a node is edited and not on create. Also moved to the top of the form so as it is obvious. And hopefully the bot likes.
#21
The last submitted patch failed testing.
#22
There's a typo in the comment as well.
#23
And 'Node type' might use a t() too I guess.
#24
Add t() to node type
Correct typo in comment
#25
The last submitted patch failed testing.
#26
New version of the patch after some comments from Morbus which are detailled below:
for the end-user UI, we call it a "Content type", not a "node type". We try to limit the usage of the word "node" to end users since it's scary and doesn't mean anything.
(see admin/content/types for an example - all "content types").
You may also want to look at the params of node_get_types - it allows you to return just the name, so that you can cut two lines of code from it and just put the call right inside the check_plain(). I think it'd be something like:
node_get_types('name', $node->type);
-------
If someone can help me shed some light on why the patch is failing I would be grateful - that was the reason why I reached out to Morbus originally but he couldn't see any obvious issue :(
#27
The last submitted patch failed testing.
#28
After speaking to Boombatower this patch applies without any syntax errors when applied to the test slaves manually
Apending patch with a suffix to trick testbot and empty patch to keep issue at CNR.
#29
The last submitted patch failed testing.
#30
If anyone has bright ideas on PHP syntax issue that would be great...I ran out of ideas and haven't gotten back to it #332967: Detect non-unix newlines
Otherwise I think this is a failed result from when slave #8 died due to Xen related issue.
#31
The last submitted patch failed testing.
#32
#33
I like this, re-rolled with unix line endings.
I'm wondering if it might be better placed in a title callback though.
Compare node/add/page to node/n/edit.
A title like "Edit page: [page title]" or [page title] (page) or something?
#34
Yes that would certainly be an valid alterntive placement for the type and I agree keeps it more in line with what currently happens on a node add.
I think the key thing here is that we the display does not affect any node based data and is clear and obvious to the user when they view the node.
#35
In the past, we've added the content type to both edit and add forms using
drupal_set_message()-- something along the lines of: "You are editing a [content type] node."Perhaps the existing message functionality can be leveraged by creating a custom message type. The message would then exist in its own div and could be customized by the theme:
$message = t('Content type: ') . check_plain(node_get_types('name', $node->type));drupal_set_message($message, 'content-type');
Output:
<div class="message content-type">Content type: Story
</div>
This would be useful if you would rather not add output to the form itself.
#36
The form item seems like a reasonable idea to me, can we have it display on one line though?
#37
-1: yet another thing that's useless to me and that i'll have to theme out to make my users happy.
#38
dmitrig01 - there's got to be some unobtrusive way you could think of to signal what content type is being edited? It isn't good usability, in my opinion, to be able to get to a form and not know exactly what the result of submitting it will be. This is exactly the case if I send you a link node/5/edit. You wouldn't know if you're beautifying a book or pimping up a page node.
#39
Yeah currently the only to know which content type you're dealing with is by going to admin/content/node and finding it that way (or /tracker, or somewhere else which isn't node/n/edit).
#40
I agree with robertDouglass and catch. In my experience, the majority of users who work with more than one content type would not be adversely affected by displaying this small piece of information. At best, usability is improved; at worst, it's extraneous information. But I can't think of an instance in which it would *harm* user experience. Please correct me if I'm wrong.
Besides, if a particular user or client is so picky that they'd want it removed, I assume they're already asking for *lots* of stuff to be changed.
#41
The last submitted patch failed testing.
#42
Re #37,38,39:
What about making it an option in the content type's submission form settings at admin/build/node-type/EXAMPLE ?
#43
No. Making things an option in the ui shifts the burden onto the user, it's basically a cop out! :)
We should be able to make a decision ourselves. And just one person's "-1" is not enough reasoning to not do this IMO.
I do think we are spending quite a lot of screen real estate on this info though. Would it be enough to have "Submit $contenttype" as the text on the Submit button maybe?
#44
Then I would have to say that it belongs in the page title like catch suggested in #33.
Whatever we do I would say -1 to forcing more clutter onto the node edit pages by default. I propose "Edit %type: %title"
#45
When I first saw the screenshot of where this puts the information it was upsetting. We add the information, but do it in a way that wastes valuable screen real estate. I imagine that's what dmitrig01 is complaining about.
So, I agree with catch in #69468-33: Make node type visible on editing form by default that this should be part of the page title. If you compare the
<title>element for node/NID and node/NID/edit they are the same. This makes it hard to distinguish which is which in bookmarks or in the URL bar autocomplete. I believe we should strive to have unique<title>elements on every page of the site.dmitrig01 - does that help?
#46
Patch and three screenshots (note especially the title at the top of the browser).
#47
#48
The only downside to the patch that I see is that $node->type is the machine readable name of the content type.
#49
Also sun is correct, this should be a title callback on the menu item...I'll try to roll a patch that does this in a moment.
#50
Also sun is correct, this should be a title callback on the menu item...I'll try to roll a patch that does this in a moment.
#51
This works. I think in IRC we're already agreeing on dropping the "this" and the colon. Let's use the italics for everyting that's not the title itself:
Edit nodetype Title of the page.
#52
The last submitted patch failed testing.
#53
So yeah, the title callback doesn't work since this is a MENU_LOCAL_TASK, it only changes the name of the tab, and not the page, or header title. :(
Here's a version without the colon and the "Edit %type" part in italic. This is also using the full human readable name of the content type as well, this means we get:
"Edit Forum topic Title" vs. "Edit forum Title" and
" Edit Blog entry Title" vs. "Edit blog Title"
#54
Here's the patch, also, patch a few tests that were breaking with this change.
#55
Looks great to me. Initially the concatenation around the t() seemed weird to me rather than just using t() with placeholders, but then I remembered that l10n folks prefer having the html outside of the string so this makes perfect sense.
I'm tempted to mark this RTBC...but will defer to dmitrig01 to find out why he was opposed.
#56
Yeah maybe its me just not knowing how placeholders work, but I didn't see a way to keep the "Edit" part of the string in italic without making the HTML external. Course this is probably bad from a theming perspective now, but I'm not sure.
Actually for translation purposes it may be better to keep the @content-type inside t() so that it will be presented to translators as a different string.(Forgot that I already did that.)#57
I like this better.
#58
Just asking... is there a reason we don't do:
t('Edit @type @title', array('@type' => $type_name, '@title' => $node->title))This seems like it would give translators more context/control.
#59
While it would give it more control/context to translators, in #51 yoroy suggested keeping everything except the title in italics. For example:
Edit Page Title
It looks like this in practice:
What webchick suggests in #58 would give:
Edit Page Title
#60
As mikey_p demonstrated, this is to give visual separation between the label and the title of the node.
My original patch used italics and a : to give the visual separation - which was overkill (in addition to awkward wording). I do agree that putting the node title into the t() gives more flexibility to translators.
So...
Edit Page This is a page
#61
Great work, folks! I went ahead and committed this. I think Dries's concern at #4 was about changing the page title, which we no longer do in this patch.
#62
Sweeet! Thanks, all.
#63
Automatically closed -- issue fixed for 2 weeks with no activity.