Problem/Motivation

As suggested in the comment https://www.drupal.org/node/2400543#comment-10461821 we need to improve the titles of the term and vocabulary pages. The titles should reflect on the edit as well as the delete page

Proposed resolution

Change the title from "Edit Vocabulary" / "Edit Term" to "Edit Vocabulary_Name" / "Edit Term_Name"

Remaining tasks

User interface changes

None.

API changes

None.

Data model changes

None.

Screenshots

Before patch Edit of Vocabulary:

After patch Edit of Vocabulary:

Before patch Edit of Term:

After patch Edit of Term:

Files: 
CommentFileSizeAuthor
#51 vocabulary-in-breadcrumb.png112.97 KByoroy
#51 vocabulary-add-term.png114.98 KByoroy
#49 Edit vocab after patch .png39.34 KBDinesh18
#49 Edit term after patch.png59.03 KBDinesh18
#49 Edit vocabulary Before patch.png39.39 KBDinesh18
#49 Edit term Before patch.png57.58 KBDinesh18
#47 2596207-47.patch4.91 KBJo Fitzgerald
#47 interdiff-46-47.txt1.37 KBJo Fitzgerald
#46 drupal-2596207-46.patch4.92 KBgaurav.kapoor
#42 EditVocabulary.PNG12.81 KBTruptti
#42 Edit_taxonomy.PNG16.11 KBTruptti
#42 Edit_forum.PNG12.22 KBTruptti
#41 edit_forum_before.jpeg9.01 KBmtodor
#41 edit_forum_after.jpeg11.8 KBmtodor
#41 edit_vocabulary_before.jpeg14.51 KBmtodor
#41 edit_vocabulary_after.jpeg12.94 KBmtodor
#41 edit_term_before.jpeg7.36 KBmtodor
#41 edit_term_after.jpeg9.04 KBmtodor
#41 2596207_41.patch4.9 KBmtodor
#41 2596207_interdiff_41_34.txt889 bytesmtodor
#34 interdiff_2596207_34_32.txt1.06 KBmtodor
#34 2596207_34.patch4.68 KBmtodor
#32 interdiff-2596207-27-32.txt768 byteschr.fritsch
#32 term_vocaubulary_edit_form_title-2596207-32.patch4.36 KBchr.fritsch
#29 After-1.png38.56 KBkrina.addweb
#27 2596207-21-27.txt3.85 KBpixelmord
#27 term_vocaubulary_edit_form_title-2596207-27.patch5.01 KBpixelmord
#24 2596207-21.patch2.93 KBbovidiu
#20 2596207-20.patch2.44 KBSagar Ramgade
#20 interdiff-2596207-20.txt1.19 KBSagar Ramgade
#13 2596207-13.patch2.4 KBameymudras

Comments

Shreya Shetty created an issue. See original summary.

Nitesh Pawar’s picture

Assigned: Unassigned » Nitesh Pawar
Nitesh Pawar’s picture

Status: Active » Needs review
FileSize
1.28 KB

Status: Needs review » Needs work

The last submitted patch, 3: 2596207-3.patch, failed testing.

Nitesh Pawar’s picture

Nitesh Pawar’s picture

Status: Needs work » Needs review

The last submitted patch, 3: 2596207-3.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 5: 2596207-5.patch, failed testing.

Nitesh Pawar’s picture

Status: Needs work » Needs review
FileSize
2.46 KB

Modified Test.

Nitesh Pawar’s picture

Priority: Normal » Minor
Nitesh Pawar’s picture

Assigned: Nitesh Pawar » Unassigned
Nitesh Pawar’s picture

Resolved the conflict happening for the forum & container add page.

ameymudras’s picture

Nitesh Pawar’s picture

Title: The edit/delete forms of term and vocabulary need better titles » The edit forms of term and vocabulary need better titles
Shreya Shetty’s picture

Status: Needs review » Reviewed & tested by the community

Its working good. Looks like RTBC to me

Nitesh Pawar’s picture

Issue tags: +rc target triage
xjm’s picture

Version: 8.0.x-dev » 8.1.x-dev
Category: Bug report » Task
Priority: Minor » Normal
Status: Reviewed & tested by the community » Postponed
Issue tags: -rc target triage +minor version target, +Usability

Thanks for the patch! This change would be a good improvement for usability.

This change would break strings, so we cannot make it any more during RC. We could improve this in a minor version, though. Reference: https://www.drupal.org/core/d8-allowed-changes

+++ b/core/modules/taxonomy/src/TermForm.php
@@ -20,6 +20,10 @@ class TermForm extends ContentEntityForm {
+      $form['#title'] = $this->t('Edit ' . $term->label());

+++ b/core/modules/taxonomy/src/VocabularyForm.php
@@ -54,7 +54,7 @@ public function form(array $form, FormStateInterface $form_state) {
+      $form['#title'] = $this->t('Edit ' . $vocabulary->label());

These are not correct uses of t(). Variables should always be added to the string using string placeholders for security and to allow them to be translated properly with context. See the documentation of the method for details.

Edit: Note that I've postponed the issue for now since 8.1.x is not open for development yet. Once it is, we can create an updated patch that improves the use of t().

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

yoroy’s picture

Issue tags: +ux-interfacetext
Sagar Ramgade’s picture

Patch updated with correct usage of t().

Sagar Ramgade’s picture

Status: Postponed » Needs review

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

pixelmord’s picture

I think this is very much needed but I would suggest to also include the vocabulary name since they are very closely tied together and you have to get the context of which vocabulary that term belongs to. Like so:

Edit term "Apple" in "Fruits" vocabulary

See the related issue for dealing with the breadcrumbs #1905370: Display which vocabulary the taxonomy belongs when editing a term

bovidiu’s picture

On the recommendation of #23 i've added : Edit term "Apple" in "Fruits" vocabulary

Status: Needs review » Needs work

The last submitted patch, 24: 2596207-21.patch, failed testing.

pixelmord’s picture

Issue tags: +dcmuc16
+++ b/core/modules/taxonomy/src/TermForm.php
@@ -15,6 +15,11 @@ class TermForm extends ContentEntityForm {
+      $form['#title'] = $this->t('Edit @name in @parent vocabulary', array('@name' =>  $term->label(),'@parent' => $term->bundle()));

I think since we're stating the type for the vocabulary name, we should also do this for the term: e.g. Edit term XXXX in YYYY vocabulary. And to mark the names a little better I would suggest to wrap the part of the text that is about the operation in s like we do that for the Node form.

So we would have:
Edit term XXXX in YYYY vocabulary
<em>Edit term</em> XXXX <em> in YYYY vocabulary</em>

+++ b/core/modules/taxonomy/src/TermForm.php
@@ -15,6 +15,11 @@ class TermForm extends ContentEntityForm {
+      $form['#title'] = $this->t('Edit @name in @parent vocabulary', array('@name' =>  $term->label(),'@parent' => $term->bundle()));

$term->bundle() will not give you the name of the vocabulary, but the machine name

I will provide a new patch for that

pixelmord’s picture

Here's an updated patch, I addressed the issues mentioned in #26 and added tests

pixelmord’s picture

Status: Needs work » Needs review

Forgot to set the status ...

krina.addweb’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
38.56 KB

@pixelmord Thanks! For your patch,It works for me same as #26.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 27: term_vocaubulary_edit_form_title-2596207-27.patch, failed testing.

The last submitted patch, 27: term_vocaubulary_edit_form_title-2596207-27.patch, failed testing.

chr.fritsch’s picture

Fixed the last failing test

Status: Needs review » Needs work

The last submitted patch, 32: term_vocaubulary_edit_form_title-2596207-32.patch, failed testing.

mtodor’s picture

Status: Needs work » Needs review
FileSize
4.68 KB
1.06 KB

Problem with test is randomization of name for Forum, that makes order of forums different randomly -> once "General discussion" is first in list, once new created forum is in front. So I made small quick solution, that first letter starts with 'f' (forum) and then it will always be in front of "General discussion".

Let's see will that work on TestBot.

chr.fritsch’s picture

Status: Needs review » Reviewed & tested by the community

Ok, lets go back to RTBC

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 34: 2596207_34.patch, failed testing.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

chr.fritsch’s picture

Status: Needs work » Needs review

Patch, should still apply. Lets test

chr.fritsch’s picture

Status: Needs review » Reviewed & tested by the community

This is still green. Retested #34

xjm’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs screenshots

Can we get before/after screenshots here? Also specifically of the Forum taxonomy forms changed as a consequence of the patch. The test changes needed to make Forum work with this patch are kinda suspicious and make me wonder if we need to add a different actual fix for Forum.

Even if we do use the current change for forum, we should probably not use the random name at all and just use an intentional test value.

mtodor’s picture

I have changed testing a bit. Instead of changing random name, I have changed weight, so that new created forum is first in list. Changing randomization of name is another topic and it's not related with this issue. I personally prefer fixed names (so that tests do not flip-flop), but other developers prefer that tests discover problems.

I have attached screenshots. I have added for term and vocabulary in summary and here are forum related ones.

Before patch Edit of Forum:

After patch Edit of Forum:

Truptti’s picture

FileSize
12.22 KB
16.11 KB
12.81 KB

Verified the patch '2596207_41.patch' in #41 on Drupal 8.4.x.The observations are as follows:
1.The patch got applied without any errors
2.The title for Edit Vocabulary is changed to 'Edit Vocabulary-name' eg . Edit Category
3.The title for Edit Vocabulary term is changed to Edit name in Vocabularyname vocabulary' eg. Edit test in Category vocabulary'
4.The title for Edit Forum is changed to 'Edit name in Forums vocabulary' eg 'Edit test in Forums vocabulary'
Attaching snapshot for reference
This can be marked as RTBC.

Truptti’s picture

Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 41: 2596207_41.patch, failed testing.

gaurav.kapoor’s picture

Assigned: Unassigned » gaurav.kapoor
gaurav.kapoor’s picture

Assigned: gaurav.kapoor » Unassigned
Status: Needs work » Needs review
FileSize
4.92 KB

Re-roll.

Jo Fitzgerald’s picture

Correct coding standards issues - "Short array syntax must be used to define arrays"

David_Rothstein’s picture

Linking to a similar issue, but for adding terms rather than editing them.

Dinesh18’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
57.58 KB
39.39 KB
59.03 KB
39.34 KB

Hello,

I have applied the patch mentioned in comment #47 (2596207-47.patch). It is working as expected. PFA screenshots.

lauriii’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: +Needs usability review
yoroy’s picture

Adding

I feel with adding the vocabulary name to the title when editing a term we might be overdoing it a bit. I don't think we should use the title to express hierarchy or relationship.

Is it at all possible to show the vocabulary name in the breadcrumb instead?

Because that is what happens when creating a new term: