Comments

duaelfr’s picture

Assigned: duaelfr » Unassigned
Status: Active » Needs review
StatusFileSize
new550 bytes

This patch is part of the #1day1patch initiative.

duaelfr’s picture

The previous patch had a few bugs :
- the message was not showing when there was no result for the first autocompleted name
- the message was not showing when two consecutive matches returned no results
- selecting the message when no results found were removing the searched string instead of letting the user correct it

Status: Needs review » Needs work

The last submitted patch, privatemsg-add_string_on_empty_autocomplete-1954496-2.patch, failed testing.

duaelfr’s picture

The same with test included (and a test only patch for TDD)

Status: Needs review » Needs work

The last submitted patch, privatemsg-add_string_on_empty_autocomplete-1954496-4.patch, failed testing.

duaelfr’s picture

And again, without a stupid copy/paste mistake...

duaelfr’s picture

Status: Needs work » Needs review

...

berdir’s picture

Status: Needs review » Needs work

Hm. Not sure, this seems to be the standard behavior for autocomplete in core? Also, the way this is implement makes it a selectable option, which is a bit weird?

+++ b/pm_block_user/pm_block_user.testundefined
@@ -54,8 +54,12 @@ class PrivatemsgBlockUserCase extends PrivatemsgBaseTestCase {
+    $test = count($autocomplete) == 1
+      && key($autocomplete) == $blocked_user->name
+      && current($autocomplete) == '<em>' . t('Sorry no matches were found for "@fragment".', array('@fragment' => $blocked_user->name)) . '</em>';
+    $this->assertTrue($test, t('User with blocked role does not appear in autocomplete results.'));

I would create separate assertions for this. This looks a) ugly and is unnecessary hard to debug when one of those assertions is no longer true.

+++ b/privatemsg.pages.incundefined
@@ -796,6 +796,9 @@ function privatemsg_autocomplete($string) {
+    $suggestions[$prefix . $fragment] = '<em>' . t('Sorry no matches were found for "@fragment".', array('@fragment' => $fragment)) . '</em>';

No need for "Sorry".

duaelfr’s picture

First, it is true that the autocomplete fields in core do not provide any message when the result is empty but it is not a good reason to do the same. Most users do not pay attention to the loading icon, so they do not understand how the field is working. Adding a message when their search does not return any result is an improvement for them.

Second, I did not find a way to return a non-clickable message so I used the already inputted content to make no difference if the user clicks the message.

There are the new patches following your advices.

ptmkenny’s picture

ivnish’s picture

Issue summary: View changes
Status: Needs review » Closed (outdated)
andypost’s picture

Status: Closed (outdated) » Needs review

D7 is not yet outdated

ivnish’s picture

Status: Needs review » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.