Problem/Motivation

Users creating a link did not know how to get the path in order to create a menu link.

Proposed resolution

stBorchert has created a patch. Menu path autocomplete is a proposed solution.

Remaining tasks

The issue of users having different content types with the same content name is still not addressed.

User interface changes

This patch changes the UI for creating menu items by adding a maximum of 10 matches in an autocomplete form element instead of a plain text field.

API changes

No API changes to date.

Original report by dcmistry

I conducted a usability study in March 2011 around content management and menu creation.

Participants were asked to create an 'About us' page and link it to the main menu. Participants who explicitly tried to create a link, could not complete the task successfully as they did not know how to get the path to create a menu link. Here is a link to a clip from the study: http://blip.tv/file/5108687

Comments from participants:
“I have to find the path. I don’t know how to find the path.” (P1)

“I am not really sure [how to find the path].” (P2)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

casey’s picture

Project: Menu Link (Field) » Drupal core
Version: 7.x-1.x-dev » 8.x-dev
Component: User interface » menu.module
dcmistry’s picture

Sorry, the above link does not work. Here is the updated one: http://blip.tv/file/5108687

yoroy’s picture

Category: task » bug
Priority: Major » Critical
Issue tags: -D7UX usability, -D8UX usability +Usability, +navigation, +d8ux, +jeremy

Thanks for the report! Yep, this is probably the core problem in our 'build navigation' ux. These results are in line with previous tests. With D8 focussing on improving site builder tools, this is a critical ux bug

Will have to dig up existing issues for this. Re-tagging for conventions already in use. New is 'Jeremy', he's our site builder persona.

catch’s picture

Priority: Critical » Major

I disagree with the critical status here, it is a very bad usability issue, but you can build a site without adding new links via the menu UI. For example adding links via the node form, links that are created from the panels or views UIs, or editing existing links created by contrib modules.

If this actually stopped people from having menus on their site, I'd agree with critical status, but it's a relatively small part of the workflow given there are multiple ways to do the same thing.

In previous testing we saw people have great difficulty figuring out that they could make menus visible via the blocks system - IMO the menu not showing up at all is worse than difficulty creating one link - although it depends how many people run into it too.

So I'm going to downgrade this to major, if you really think this is a blocker compared to other severe usability issues in the queue, then move it back of course. For me I'd rather we reserved critical issues for things that actually make Drupal unusable (for example if the standard profile shipped without a link to the administration area, or a visible login link, both nearly happened at one point). It would be good to document how the critical priority relates to usability issues on http://drupal.org/node/45111, usability, accessibility (and performance) tend to get treated a bit differently on that scale compared to other issues but aren't mentioned there yet.

yoroy’s picture

Fair enough. We'll see what status this has after next weeks test :)

stBorchert’s picture

Shameless self-promotion: maybe http://drupal.org/project/mpac could give a direction how to solve this problem?

Bojhan’s picture

@stBorchert Ideally it also auto completes based on Node title? That would be ideal. Now you still have to know the url structure (kind of)

stBorchert’s picture

@Bojhan: yes, it autocompletes on node titles (and on URL aliases).

If you like to, I'd create a patch for D8 so you can test this.

Bojhan’s picture

That'd be great, I probally have to work out how git works then :). I would like to see how you solved that visually

stBorchert’s picture

This patch makes the path textfield an autocomplete that lists matching node titles for entered characters.
https://img.skitch.com/20110607-xt6dxeyehc7n1xqrbf2tg9jj77.png

After clicking on the first title the path of the node is inserted: https://img.skitch.com/20110607-knukrmr5p99hta245rtq6ts4xy.png

Unfortunately this is still affected by #93854: Allow autocompletion requests to include slashes which breaks autocomplete when using a slash (it happens even if you leave the field in some cases).

dcmistry’s picture

This is great stBorchert.

My only concerns are:
- What if the user has different content types (like Basic Page, Poll, Content Types, etc) with the same content name. In this solution, the user will have no way to know which is what
- Also, wondering after clicking on the title of the path, inserting node might confuse the user. That brings the bigger question, why does the user need to know node/X at this point, anyway?

sun’s picture

Title: Participants did not know how to get the path while creating a menu link » Add autocomplete for menu path when creating a menu link
Category: bug » feature
Priority: Major » Normal
Status: Active » Needs review

Seriously?

Bojhan’s picture

Title: Add autocomplete for menu path when creating a menu link » Participants did not know how to get the path while creating a menu link

Again, yes. Because menu creation is fundamental to the UX of a CMS. Given that we have two major issues around it, they should be fixed asap - however, this is "a" approach - some I am not sure it would quallify as the major - http://drupal.org/node/1101600 is pretty much duplicate/related.

stBorchert’s picture

What if the user has different content types (like Basic Page, Poll, Content Types, etc) with the same content name.

This is indeed a major problem of using this type of autocomplete. It even gets more problematic if you have more than 10 nodes with the same title. The current implementation lists 10 items only so you have no chance to select the 11th.

It would be great if we could provide more information about the node in the popup (e.g. the type and the path). Maybe #125231: Enhance autocomplete feature and #675446: Use jQuery UI Autocomplete will be a good start to make this possible.

why does the user need to know node/X at this point, anyway?

Currently users need to know the path on different places. Maybe we can omit the internal path on the whole site; doesn't know if this will work.
Additionally you can enter not only node paths but path to every site within the installation. It would be inconsequent if you enter a title for nodes and the internal path for admin pages (for example).

Btw.: this problem doesn't affect menu item creation only. Look at shortcuts, url aliases and the default frontpage setting. All of them are using an internal path.

catch’s picture

Right now if you add a menu link as an alias, this works, but when it's saved it is converted to the system path. When you go back to edit the link you'll see the system path since that's what gets stored.

It may be possible when rendering the form, to look up the alias for the system path, and show this in the form, but if you added the system path there on purpose, then it would equally be showing you something different to what you entered.

I don't think we can completely hide the path alias / system path divide from administrators in the menu system, but trying that would be a separate issue if you think it's that bad of a problem.

webchick’s picture

Category: feature » task
Priority: Normal » Major

There is absolutely no way on this earth that this is a "normal" "feature request". This was the cause of help desk calls in a ridiculous number of participants.

Splitting the difference at "major task".

dcmistry’s picture

I do prefer this: http://drupal.org/node/1101600
I am sure, stBorchert you must have looked at this, thoughts?

stBorchert’s picture

@dcmistry: yes, I know this issue. And I like especially the mockups of #1101600-13: Users need to be able to select from list when adding menu items to a menu :).

Maybe we should reference here to the issue over there and try to push it? Imo, #1101600: Users need to be able to select from list when adding menu items to a menu will solve this issue, too.

tamanna-freelancer’s picture

I have an idea for this.When we submit the node we can have a message displayed "Path for this node is 'node/1'".

So when the user creates the node and sees this message he will know the path.

dcmistry’s picture

@stBorchert: Yeppers! Good idea.

@tamanna: The problem with that is that we are going against "recognition rather than recall" principle of usability. We don't want the user to have to "recall" and add to their cognitive load. Instead we want to give the user the option to "recognize".

Resource (Look under "Recognition rather than recall"): http://www.useit.com/papers/heuristic/heuristic_list.html

xjm’s picture

Tagging issues not yet using summary template.

gurlinthewurld’s picture

Issue summary: View changes

Status to date.

gurlinthewurld’s picture

Issue summary: View changes

Status to date.

invisibleink’s picture

Re-rolled patch from #10 to work with new core file structure.

invisibleink’s picture

Issue summary: View changes

Status to date.

Ron Williams’s picture

Issue summary: View changes

Removed status in body

Ron Williams’s picture

This patch includes a cleaned up version of the patch and simpletest for the changes. invisibleink and I worked together during the core code sprint at DrupalCon Denver.

xjm’s picture

Status: Needs review » Needs work
Issue tags: +Novice, +Needs screenshots, +Needs manual testing

Awesome work on this, @invisibleink and @Ron Williams! That looks like a good test. Hope you enjoyed the sprint! :)

I reviewed the patch and found a few things to fix. (Most of these were also issues in the previous patch in #10.) Adding the "novice" tag because most of them are fairly straightforward fixes now that the initial test method is in place.

  1. +++ b/core/modules/menu/menu.admin.incundefined
    @@ -684,3 +685,29 @@ function menu_configure() {
    +/**
    + * Helper function for autocompletion.
    + */
    

    This docblock should start with a third-person verb and be a little more detailed. We should also add the @param documentation. See taxonomy_autocomplete() for an example of a docblock for a similar function. (Note that the "Path: taxonomy/autocomplete" line in that docblock follows an old standard and should not be included.)

  2. +++ b/core/modules/menu/menu.admin.incundefined
    @@ -684,3 +685,29 @@ function menu_configure() {
    +function menu_autocomplete($title_typed = '') {
    +  $title_matches = array();
    

    I think we need to use the func_get_args() pattern that we recently added to taxonomy_autocomplete() when fixing #93854: Allow autocompletion requests to include slashes. See also #1492378: Document proper use of #autocomplete_path for slashes and http://api.drupal.org/api/drupal/developer%21topics%21forms_api_referenc....

  3. +++ b/core/modules/menu/menu.admin.incundefined
    @@ -684,3 +685,29 @@ function menu_configure() {
    +    $query->addTag('node_access');
    +    ¶
    

    There's a bit of trailing whitespace on the blank line here.

  4. +++ b/core/modules/menu/menu.testundefined
    @@ -29,6 +29,18 @@ class MenuTestCase extends DrupalWebTestCase {
    +   * Test if autocomplete returns user created node for menu path
    

    This should begin with a third-person verb ("Tests") and end with a period. Also, it's best to add articles so that it is more readable. I'd suggest:
    Tests whether the user-defined node path alias is returned via autocomplete.

  5. +++ b/core/modules/menu/menu.testundefined
    @@ -29,6 +29,18 @@ class MenuTestCase extends DrupalWebTestCase {
    +    $result = $this->drupalGet('menu/autocomplete/' . 'invis');
    

    I think we can just say 'menu/autocomplete/invis' here rather than concatenating the two strings.

    Can we expand the test coverage for a node title alias that includes slashes? (Ideally, actually, I'd like to see the test coverage expanded to include the various cases covered in the taxonomy autocomplete tests. See TaxonomyTermTestCase::testNodeTermCreationAndDeletion() and TaxonomyTermTestCase::testTermAutocompletion().)

  6. +++ b/core/modules/menu/menu.testundefined
    @@ -29,6 +29,18 @@ class MenuTestCase extends DrupalWebTestCase {
    +    $this->assertIdentical($result, $expected, t('The autocomplete path returned the proper JSON object when acessing autocomplete URL'));
    

    Typo here ("acessing"). Let's also add a period at the end of the message. Finally, note that the assertion message here does not need to use t() (since it is only displayed to developers and no translators are going to translate it). Reference: http://drupal.org/simpletest-tutorial-drupal7#t

When the patch is updated, can we also upload the automated tests in their own file as the first attachment, and the complete patch as the second? This will expose the failures of the tests when the feature is not working.

Finally, some thoughts the patch functionality overall:

  • I think that #1101600: Users need to be able to select from list when adding menu items to a menu is probably a good idea, although we could probably also do both.
  • It would be good to get some manual testing of the patch for usability, etc. For example, I'd be interested to know if the autocomplete "gets in the way" when the user wants to enter a plain path alias, or if entering both the title and the path in the same field seems intuitive.
  • It would be good to have some manual testing of the feature when there are multiple nodes, including titles that contain various punctuation like slashes, single quotes, double quotes, and commas, plus combinations thereof.
  • It would also be good to have a few screenshots of the behavior for reviewers, so that we can see the new field description and some samples of the autcompletion in context.
  • It also looks like #11 (regarding multiple nodes of the same title) still needs to be addressed.
xjm’s picture

Issue summary: View changes

updated ui changes

Dig1’s picture

Well, after much hand-holding from xjm I have managed to fix the function summary for point #1 in comment #24, but the other points aren't addressed yet...

Dig1’s picture

Status: Needs work » Needs review

Dig

webchick’s picture

Great job, Dig1! :)

sun’s picture

Status: Needs review » Needs work
+++ b/core/modules/menu/menu.admin.inc
@@ -694,3 +695,29 @@ function menu_configure() {
+function menu_autocomplete($title_typed = '') {

The reality is that this is a menu_node_autocomplete().

1) Perhaps we should add the $entity_type as first argument, so that the arguments effectively are 'node/abc'.

2) Perhaps it makes even more sense to move this menu callback into Entity module, rename it to entity/autocomplete, and use an EntityFieldQuery to retrieve the results and leverage entity_load_multiple() as well as $entity->uri() to get the corresponding paths.

That sounds like a trivial change to do, and immediately makes this much more re-usable.

+++ b/core/modules/menu/menu.test
@@ -31,6 +31,18 @@ class MenuTestCase extends WebTestBase {
+  function testMenuAdminNodeTitleAutoComplete() {

This method should go into an own MenuAutocompleteTestCase that depends on nothing else than node and menu module.

stBorchert’s picture

Status: Needs work » Needs review
FileSize
4.79 KB

Here is an updated version of the patch using the more generic approach suggested by sun.
I'm not sure about 'access arguments' => array('access content'), in the menu definition, maybe we could you a more generic permission here.

Bojhan’s picture

Can this get a screenie?

stBorchert’s picture

It did not change until comment 10:

sun’s picture

Issue tags: -navigation, -Needs manual testing, -jeremy

Thanks. Looks good, except:

1) Missing ->addTag('node_access') and ->addTag('translatable').

2) The existing comments in the callback can be removed. Instead, we need new comments that explain why the conditional structures are there and what they are supposed to do. It looks like you're skipping entity types that do not have a label defined, which sounds sane, but explain that, and why.

3) You're using the procedural entity_*() functions, but I think we want to use the $entity->methods() now. (for uri and label)

4) Since we still don't have a generic entity access yet (but that will hopefully follow very soon), I'd recommend to replace the access callback with a new entity_autocomplete_access($entity_type), which manually checks for the basic user permissions of hard-coded entity types; i.e., "access content" for type node, "access user profiles" for type user, "access comments" for type comment. Hard-coding is all we can do right now, but makes the most sense for this issue and patch. As soon as we have a generic entity access API, this can be replaced. (And if it doesn't happen for any reason, there's no harm in having at least this entity autocomplete functionality for core entities.)

stBorchert’s picture

Ok, here is a new one:

* added tags to the query
* tried to explain why we can't query entities without a label key
* changed procedural calls to uri and label
* added custom access callback

Some notes:
I didn't add "file" to the access callback because access to files isn't really simple without knowing which the user tried to access.

For now the autocomplete results are somehow bound to menu because the uri of the entity is returned as key. What if someone likes to use entity_autocomplete() and needs the entity-id as key?
Do you see any chance of making this more flexible?

sun’s picture

Status: Needs review » Needs work

re: paths vs. IDs: Let's care for that later in a separate issue, when actually needed.

+++ b/core/modules/entity/entity.module
@@ -21,6 +22,89 @@ function entity_help($path, $arg) {
+ * @param $entity_type

Let's add the data type "string" after @param here.

+++ b/core/modules/entity/entity.module
@@ -21,6 +22,89 @@ function entity_help($path, $arg) {
+      return FALSE;
+      break;

1) No break after return.

2) There should be a blank line between switch cases, unless a case "falls through" to the next.

+++ b/core/modules/entity/entity.module
@@ -21,6 +22,89 @@ function entity_help($path, $arg) {
+ * Page callback: Outputs JSON for entity autocomplete suggestions.
+ */
+function entity_autocomplete($entity_type, $title_typed) {

Missing phpDoc @params + @return.

Let's also add a @see to the menu access callback and vice-versa.

+++ b/core/modules/entity/entity.module
@@ -21,6 +22,89 @@ function entity_help($path, $arg) {
+  if ($title_typed != '') {

This should be a strict comparison; i.e., !==, because searching for 0 should be possible.

+++ b/core/modules/entity/entity.module
@@ -21,6 +22,89 @@ function entity_help($path, $arg) {
+          if ($entity_uri) {
+            $matches[$entity_uri['path']] = check_plain($entity->label());

The condition has to be !empty($entity_uri['path'])

stBorchert’s picture

Status: Needs work » Needs review
FileSize
6.11 KB

tadaa ...

sun’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
11.33 KB
3.59 KB
5.66 KB

Thank you!

Attached patch cleans up some minor things, and most notably, removes vocabularies from the possible entity types, since we're very likely going to remove them from the entity concept altogether in #1552396: Convert vocabularies into configuration.

See interdiff.txt.

Also adjusted the form element description; new screenshot:

menu-link-form.png

Since these are very minor changes, I'm calling this RTBC.

tim.plunkett’s picture

Status: Reviewed & tested by the community » Needs review
+++ b/core/modules/entity/entity.moduleundefined
@@ -56,9 +56,7 @@
-    case 'taxonomy_vocabulary':
-      // There is no specific permission to allow access to vocabularies or
-      // terms so return TRUE.
+      // There is no specific access permission for terms so return TRUE.

This isn't a little premature?

+++ b/core/modules/entity/tests/entity.testundefined
@@ -251,7 +251,6 @@
 class EntityAutocompleteTestCase extends WebTestBase {
-
   public static function getInfo() {

I thought we were supposed to have those lines...

+++ b/core/modules/entity/tests/entity.testundefined
@@ -263,29 +262,20 @@
-    $modules = func_get_args();
-    if (isset($modules[0]) && is_array($modules[0])) {
-      $modules = $modules[0];
-    }
-    $modules[] = 'node';
-    // Ensure node module is loaded and configured.
-    parent::setUp($modules);
+    parent::setUp(array('node'));

I'm going to trust you on this one :)

+++ b/core/modules/entity/tests/entity.testundefined
@@ -263,29 +262,20 @@
-    $this->assertIdentical($result, $expected, t('The autocomplete path returned the proper JSON object when acessing autocomplete URL'));
+    $this->assertIdentical($result, $expected);

Why remove the assertion text?

sun’s picture

1) I'm 99% confident that vocabularies will not remain as entities. In the rather unlikely case I'm wrong, the taxonomy_vocabulary case can be easily added back later.

2) No idea whether we have a standard for blank lines, and if we happen to have one, then it's totally not applied anywhere.

3) That $modules syntax is only needed for base test cases being extended by others. This test case contains a test method, so it cannot be extended by others in the first place.

4) Custom assertion messages are hiding actual bugs and make debugging unnecessary complex in case of test failures. They only make sense for simple assertions, which do not compare two values against each other. @tim.plunkett mentioned in IRC that we seem to have defined a standard somewhere in test handbooks that ask for custom assertion messages. That standard needs to be corrected.

Bojhan’s picture

Status: Needs review » Reviewed & tested by the community

Lets get this back to RTBC, looks like sun addressed tim's concerns - if not feel free to move it back.

Dave Reid’s picture

+++ b/core/modules/entity/entity.moduleundefined
@@ -21,6 +22,99 @@ function entity_help($path, $arg) {
+        ->addTag('node_access')

I'm a little confused why we aren't using $entity_type . '_access' instead of just 'node_access'?

Dave Reid’s picture

Overall I find the menu item and callback ambiguous. The autocomplete is actually for entity URI, not just the entity ID, or title, etc. Something like entity/autocomplete/uri/% would have been more expected (also if entity type is a required argument, not sure why it's isn't listed with a % in the menu item path). We could leave that as a follow-up for the sake of getting this improvement in.

Dave Reid’s picture

I also don't see any kind of protection that this autocomplete should not work if the entity type has no URI callback available.

Everett Zufelt’s picture

Has this proposed UI change been reviewed for accessibility?

Primarily does it work well for keyboard only, magnifier and screen-reader users?

Thanks :)

Bojhan’s picture

@Everett This is simply autocomplete.

catch’s picture

This doesn't include the necessary changes from #1492378: Document proper use of #autocomplete_path for slashes to handle slashes.

Also I see no reason not to specify the parameters in the hook_menu() definition.

In terms of the menu UI change itself - is it really OK to have an autocomplete for nodes when menu links can point anywhere else too? Have people tested this trying to put arbitrary paths that match node titles or similar?

catch’s picture

Status: Reviewed & tested by the community » Needs work
hefox’s picture

(Just stumbled on this issue, but the statement of the problem for remaining tasks bothered me:

- What if the user has different content types (like Basic Page, Poll, Content Types, etc) with the same content name. In this solution, the user will have no way to know which is what

Unless d8 has changed it, entity name/node title/etc. isn't necessarily distinct within a given bundle type/content type, e.g. this isn't related to different content types just made more common? The title could be instead "Title (aliased path)" as that will be distinct.

#777578: Add an entity query access API and deprecate hook_query_ENTITY_TYPE_access_alter() is an issue on entity access. There may be another I don't know about. Not sure if good idea, but, there could be an issue broken off that to make the basic permissions (access content, etc.) be defined with the entity type definition, which is simpler to develop, which likely will mean it'll could get into core sooner for patches like this to use?

On the node vs. other paths that catch mentioned (hm, menu doesn't depend on node?): that was my first thought when reading this issue and the solution, however it seems like "better than nothing, and other things would be more complicated" type deal (from my brief read)?.

JvE’s picture

My 2cts:

I doubt that autocomplete is going to improve menu link creation. Mainly because of the dissonance between the displayed suggestions (labels) and the used value (a path) and the fact that you can enter two different types of input (paths & node titles) into one element.
I'd add it as a separate box behind the path field. Maybe called "use node path" or something.

If autocomlete is an improvement then I feel a custom 'node title'->'uri' node_title_to_path_autocomplete is the way to go.

A generic entity autocomplete functionality is way too big a monster to try to tackle in this context.
There are usecases for such a function matching labels to ids, searching on other properties than labels, options like published/unpublished, matching CONTAINS vs STARTS_WITH, limiting by bundle, duplicate vs unique labels, etc.

klonos’s picture

I doubt that autocomplete is going to improve menu link creation...

It most certainly is! The proposed solution here might not be perfect (it won't please *everybody* and there'll always be room for improvements), but it is far better than the plain text field we currently have. The UX is definitely improved.

That being said, I too would rather see #1101600: Users need to be able to select from list when adding menu items to a menu be implemented, but I see no patches - only mockups there. So, perhaps we should do this and if the other issue moves on, then we can swap features/functionality.

klonos’s picture

Issue summary: View changes

Updated issue summary.

klonos’s picture

...btw, perhaps both the issue of users having different content types with the same content name as well as the WTF of entering a title and having the field replace it with a path could be solved by using the jQuery UI in its "Custom data and display" format:

Notice how there can be a different value/label/description:

...
<script>
	$(function() {
		var projects = [
			{
				value: "jquery",
				label: "jQuery",
				desc: "the write less, do more, JavaScript library",
				icon: "jquery_32x32.png"
			},
			{
				value: "jquery-ui",
				label: "jQuery UI",
				desc: "the official user interface library for jQuery",
				icon: "jqueryui_32x32.png"
			},
			{
				value: "sizzlejs",
				label: "Sizzle JS",
				desc: "a pure-JavaScript CSS selector engine",
				icon: "sizzlejs_32x32.png"
			}
		];
...

I mean, it is planned for replacing our custom autocomplete with the one jQuery UI includes after all (#675446: Use jQuery UI Autocomplete)

In the example image/code above think of:

- value being the actual path we need to store in the db
- label being the "pretty" node title we want the user to see in the form
- desc perhaps being the full URL or a hint that the node is an article/page/whatever

Gábor Hojtsy’s picture

FileSize
3.51 KB

Bojhan marked #1879816: Add node autocomplete to help path specification as a duplicate. I posted a node-only patch there, which is I believe identical to functionality to the above patch's actual implementation. Although the above patch introduced generic autocomplete's for entity types, it only used node based autocomplete (and had bugs in the implementation).

Also re-posting Wim Leers' comment from there:

Ideally we'd support this for more than just nodes (which would be confusing to users since it's so limited in scope). The first thing that comes to mind is that every Entity that is accessible through an URL should be "autocompleteable". If others think that makes sense, I think the next step could be to let EntityInterface (or ContentEntityInterface not only extend ComplexDataInterface, AccessibleInterface, and TranslatableInterface, but also a new AutocompleteInterface? Though in that architecture, it might be hard for the path module to also be integrated?

I'm just posting this patch for posterity, the approach probably needs a bit of rethinking based on the above, a potentially better design in #1879816: Add node autocomplete to help path specification and Wim's more up to date comments.

Gábor Hojtsy’s picture

@tkoleary proposes this design to have a "link" field which works (at a minimum) as a node autocomplete and a separate path field that allows people to specify a specific path, so you can comfortably specify paths that are not titles, and the title is visible even if the path is filled in (on later edits even):

Pathautocomplete.png

Implementing this specifically would require more intricate autocomplete behaviours, where the autocomplete has effect on a separate field. Also server side submission for the autocomplete field to cross-validate or ignore the text field on submission and fill in based on the path on edit.

(Ignore the layout of the form fields for this issue).

sun’s picture

#52 looks rather confusing to me. Primarily, because there are two controls for the same/identical input value.

I guess it would make more sense if the Path input field would be right next to the Title input, and the autocomplete would come last, but it would essentially just be a "search widget" that is completely decoupled from the form, and which only exists to populate the Path input field as a pure UI helper.

In other words, if you'd use the Path input field directly, there's no magic and autocomplete involved at all, and you fill in or edit the value directly. If you don't know the value, then you consult the search widget right next to it, which allows you to search, list, and retrieve available paths; clicking/selecting one populates the Path input field. You can search at any time without affecting the form values in any way (unless you select a search result).

I'm not sure whether we want to pursue that direction, but if so, then I'd approach it that way.

Otherwise, I'd rather go back to a combined input field with autocomplete behavior — in an ideal world, the autocomplete would be able to search for both entities of all types AND paths AND pages (e.g., a views page); i.e., a super smart URL widget that's simply able to figure it out.

That is, because the limitation on nodes (or entities) never made any sense to me in the first place — only very small or smaller sites actually put direct links to entities into their menus — any site that's only a little bit more complex needs to link to site sections, views, and layouts/panels as entry level pages in its menus - and the autocomplete doesn't work at all for that.

Gábor Hojtsy’s picture

@sun: the problem with the autocomplete in one field is once you pick a node (or anything your super-smart URL widget would support), you loose the title you types, you only see the path and no info on what is behind it. The two separate fields were proposed to keep the URL and title of the page (if known) visible, so you can be more comfortable you are doing the right thing. Ideally I'd imagine if you put in a valid path, we could reverse-lookup a title for it, if possible (with the knowledge behind the super-smart URL thing) and vice-versa if you enter a title, we'd look up the path. Drupal needs the path but users are more comfortable with knowing what is behind that. node/12 is not very intuitive. I'm not sure using one field as both a title or path input is very intuitive, I don't think people would find it natural that you type in a title in a "path" field, and then it suddenly disappears and replaced with something entirely different...

stBorchert’s picture

FileSize
26.06 KB

Hm, what about having the autocomplete display the path (as it would now) and print the current title of the linked item right after the autocomplete?
automcomplete with title

tkoleary’s picture

"Btw.: this problem doesn't affect menu item creation only. Look at shortcuts, url aliases and the default frontpage setting. All of them are using an internal path."

Excellent point. The solution should cover all of those as well.

tkoleary’s picture

@Sun I think Gabor summarized my thinking pretty well above. The users expectation is that they should be able to link things together based upon the names that they have assigned to those things. The fact that there are things in Drupal you can link to that are created automatically necessitates the "path" field, but the primary (and first after "title") should be "link" (as in the image attached by Gabor).

The case that I have not yet designed for is where there are multiples of the same name. I am working on that now.

tkoleary’s picture

@stBorchert "Hm, what about having the autocomplete display the path (as it would now) and print the current title of the linked item right after the autocomplete?"

The users expectation is that they will be able to type the name and find what they want, not the path.

sun’s picture

I've the impression that #53 was misinterpreted.

The main aspect of it was rather technical; essentially we'd leave the path field as path field in the form. However, we enhance the user interface through a pure front-end, JS-driven "PathFinder" search widget that gets attached to the path field (but does not replace it):

Path:                       Search:
+-----------------------+   +--------------------+ 
| node/123              | « | uni                | [ Use this path ]
+-----------------------+   +--------------------+---------+
                            | Blog post: My Unicorns       |
                            | blog/2012/my-unicorns        |
                            +------------------------------+
                            | Video: Universal Sol...      |
                            | videos/universal-soldiers    |
                            +------------------------------+
                            | View: Unisex t-shirt rates   |
                            | store/rates/shirts           |
                            +------------------------------+

Without JS, there's simply no search widget visible and attached.

Bojhan’s picture

How does this design deal with not known-item seeking? For example, pages with very similar labels (medicinal sites), Noyz had a design for this in #1101600: Users need to be able to select from list when adding menu items to a menu. I definitely agree, title is leading.

Brainstorming, @sun has a good point, that we could just get away with only an auto complete field with a [edit] to change the URL (I imagine the select list item would be two rows, one row the title and the second row the path, so you see what url and title you select). Crosspost! Essentially what @sun suggests, but without the two input fields - I think that would be confusing. We shouldn't give the user an option to do two, just the one. Where you can change the URL, machine-name-like, or less silly put the title on top let it auto-search and just fill in path :)

tkoleary’s picture

@Bojhan. I like that idea. It maintains the principle of giving priority to the human readable name, solves the problem of multiple items with the same name and also leaves room for the advanced user. I also like that its similar to the new machine name pattern which I loveI. I'll re-work the design.

On the browse for content suggestion I think for the moment what we have covers the primary use case of "I want to create a link to a page I know the name of", but I have always liked Jeff's design and I think it would be a great stage two enhancement.

Bojhan’s picture

@tkoleary sounds good like followup, the browser of jeff. I will make one once this sucker gets in :)

Gábor Hojtsy’s picture

Yeah I think the proposed browser in #1101600: Users need to be able to select from list when adding menu items to a menu is a lot more invasive, eg. it opens a popup above the editing form, which would be pretty bad to be opened above the supposedly small modal... I see that has lot more features, filtering, meta-info on found things, etc. I'm not sure we need that extent here?

tkoleary’s picture

@Gábor Hojtsy Yes, I think you're right, it's too ungainly for a modal. If we do add this at a later point I think it should probably work like this:

User clicks "browse" we unload the form, bring the user to the full "search content" form. Once they search, each results row has an operation "add to menu" which gets the name and url, redirects to the referring url (the relevant add link form) and pre-populates the name and url values.

Easy. ;)

Gábor Hojtsy’s picture

Ok then let's figure out some immediate direction we can go with so we can get down to implementation. This is very much blocked on us not knowing what to implement.

tkoleary’s picture

FileSize
162.92 KB

@Gábor Hojtsy I think that this (below) is the MVP. The "browse" flow discussed above is a nice-to-have but not essential now.

tkoleary’s picture

FileSize
161.31 KB
tkoleary’s picture

FileSize
160.36 KB

Ugh. No way to revise an uploaded image.

link flow

sun’s picture

Issue tags: -Usability, -Novice

Thinking through the technical aspects of this, I'm relatively sure that this widget/facility can only be provided by Search module.

That is, because this is about router paths, but not only about the statically registered router paths. Every available dynamic path needs to be translated into all possible options.

We need an index of "every possible page on your site", including title + path + ideally some more meta data to enrich the widget; e.g., the entity type as well as the bundle in case of entities.

It is not possible to generate this listing dynamically (on demand), as that would be too time-consuming (you'd pretty much have to load all data that exists in the system, in order to filter it, and also in order to check for access).

In turn, we're talking about a search index and search results.

webchick’s picture

Hm, that's an interesting approach. We also came to a similar conclusion that it'd have to be a lookup table because calculating this on keypress would be insane.

So maybe the form could just expose path in the same stupid way it does now if Search module is disabled, but if it's enabled, it could provide the "magical lookup field of unicorns and rainbows"?

webchick’s picture

Issue tags: +Spark

Tagging.

YesCT’s picture

Issue tags: -Needs screenshots, -Spark

I *think* screenshots of previous patches are not needed right now, and waiting on a patch for some new proposed approach.
Add the tag back when this is ready for new screenshots.

YesCT’s picture

Issue tags: +Spark

I did not mean to remove Spark. bad tags!

webchick’s picture

This seems to be a duplicate of #1101600: Users need to be able to select from list when adding menu items to a menu but since that issue has no patch and this one does, I closed it. It does however have some of the same discussion, plus some additional mocks and a pointer to https://drupal.org/project/content_menu at #1101600-55: Users need to be able to select from list when adding menu items to a menu which looks like it does a lot of what's asked for here.

webchick’s picture

Issue tags: -Spark

Also, I'm going to untag this from Spark on purpose because it seems unlikely we'll get to this in D8, and this wasn't part of any code we specifically introduced.

webchick’s picture

Issue tags: +Usability

And adding the Usability tag.

webchick’s picture

Issue summary: View changes

...no need to keep a non-working link in the issue summary while we have a working one.

kattekrab’s picture

Is it worth revisiting this? Would be a big win for users.

Wim Leers’s picture

Since #1101600: Users need to be able to select from list when adding menu items to a menu was closed as a duplicate in favor of this one, I have to remark that this issue has no mention of the "Content Menu" module, which was explained/presented by klonos at #1101600-55: Users need to be able to select from list when adding menu items to a menu. Copy-pasting what he wrote verbatim:

I don't see Content Menu mentioned in this issue at all. Here's how it does things...

Adding a menu item that links to existing content (other options include a plain URL/path, a "Dummy" no-content node, or New {content_type}):

Adding existing content

Here's how you edit existing menu items content directly from the menu management form and how you link to a new page. All content types configured to allow menu items are added in the "Target" drop-down menu in the form of "New {content_type}" entries:

Edit existing menu items

The little pencil icon next to each menu item label allows to edit it in place.

And here are the actual steps in creating a new menu item that links to a page and also creating that page:

Adding a menu entry that links to new content.

Wim Leers’s picture

webchick’s picture

Status: Needs work » Closed (duplicate)

While this issue is older, trying to do a "fresh start' on this topic with all of the various ideas over at #2407913: Discuss/define the minimal UX for adding menu links to entities.