search design details #1606

jancborchardt · 2016-10-01T16:16:07Z

Some search design details I did a while ago. Including:

fixing wording
use correct styling/elements
cut off leading slash

Please review @nextcloud/designers @nextcloud/javascript :)

eppfel

Actually for a user it is not transparent, why there are two different "no results" messages and it looks like a jump frame.

eppfel · 2016-10-01T16:42:25Z

core/search/js/search.js

@@ -217,7 +217,7 @@
 					$status.addClass('emptycontent').removeClass('status');
 					$status.html('');
 					$status.append('<div class="icon-search"></div>');
-					$status.append('<h2>' + t('core', 'No search results in other folders') + '</h2>');
+					$status.append('<h2>' + t('core', 'No search results in other folders {filter}', {filter:this._filter}) + '</h2>');


This is not substituted and will be shown as {filter} on my end.

Isn't it supposed to be ('... folders %s', this._filter) – or something like this?
Edit: this is for php, I guess.

Okay, I see, you might just need the last two parameters, maybe?
..., {filter:this._filter}, null, {'escape': false}) as in filelist.js

cc @nextcloud/javascript can you help in this regard?

@jancborchardt is this supposed to display the searched string?

Because this._filter is undefined for me. I think we should pass the query in the function arguments if that's what you were planning! :)

this is undefined, will fix

Signed-off-by: Joas Schilling <coding@schilljs.com>

skjnldsv · 2016-10-05T11:53:58Z

@nickvergessen Thanks for the fix!

I don't find it really clear. Should we put some more details? Like "No search results in other folders for «tests»

nickvergessen · 2016-10-05T11:54:05Z

It still looks like the search icon and the text are jumping down/up at 2 to 3 characters

skjnldsv · 2016-10-05T11:57:40Z

Small question, why don't I have the same output when searching small queries?

Edit: oooh, is this what you were talking about Joas? 😛

eppfel · 2016-10-05T12:03:20Z

@nickvergessen @skjnldsv That's what I addressed in my review. One is handled in core and the other in files???

skjnldsv · 2016-10-05T12:06:51Z

There's something strange here indeed! :)

MorrisJobke · 2016-10-05T21:51:30Z

There's something strange here indeed! :)

It is by intention: up to 2 charaters it only filters the current view and starting with 3 characters it also triggers a search in all other folders (parents and childs).

MorrisJobke · 2016-10-05T22:03:38Z

@jancborchardt Could I ask you to respond to all the open questions.

skjnldsv · 2016-10-06T11:57:05Z

@MorrisJobke Why though?

jancborchardt · 2016-10-06T15:03:26Z

Yeah ok, that’s weird. To clarify, any search error should only have that one large line of text. Anything else is too complex for a simple search error. Will get on it.

jancborchardt · 2016-10-06T15:14:42Z

Btw @ChristophWurst here too I would like to bold the search string like we did the folders for the mail error messages: nextcloud/mail#57 (review)

Can you help me with that? Cause there’s no tag and endtag like in Mail.

skjnldsv · 2016-10-06T15:21:30Z

core/search/js/search.js

@@ -217,7 +217,7 @@
 					$status.addClass('emptycontent').removeClass('status');
 					$status.html('');
 					$status.append('<div class="icon-search"></div>');
-					$status.append('<h2>' + t('core', 'No search results in other folders') + '</h2>');
+					$status.append('<h2>' + t('core', 'No search results in other folders {filter}', {filter:lastQuery}) + '</h2>');


@jancborchardt since you're using append here, we can use directly into the string.
$status.append('<h2>' + t('core', 'No search results in other folders for <strong>{filter}</strong>', {filter:lastQuery}) + '</h2>');

Won’t that be a problem with translators again because they often mess up the tags? I thought that was a problem cc @LukasReschke

<fett>{filter}</fett>

🙈

Oh yes it will indeed!
We should proceed th same way as ChristophWurst.

@jancborchardt
var error = t('core', 'No search results in other folders {tag}{filter}{endtag}', {filter:lastQuery}).replace('{tag}', '<strong>').replace('{endtag}', '</strong>'); $status.append('<h2>' + error + '</h2>');

@skjnldsv can you add that as a commit directly on this branch? You are a Nextcloud member so you should be able to do that ;)

skjnldsv · 2016-10-06T15:22:40Z

apps/files/js/filelist.js

@@ -2427,7 +2427,7 @@
 				$('#searchresults .emptycontent').addClass('emptycontent-search');
 				if ( $('#searchresults').length === 0 || $('#searchresults').hasClass('hidden') ) {
 					this.$el.find('.nofilterresults').removeClass('hidden').
-						find('p').text(t('files', "No entries in this folder match '{filter}'", {filter:this._filter},  null, {'escape': false}));
+						find('h2').text(t('files', "No results in this folder for {filter}", {filter:this._filter},  null, {'escape': false}));


Same here, you can use html instead of text if you want to add <strong>

find('p').html(t('files', "No entries in this folder match '{tag}{filter}{endtag}'", {filter:this._filter}, null, {'escape': false}).replace('{tag}', '<strong>').replace('{endtag}', '</strong>'));

Same here ;)

You mean {tag} and {endtag}?

@skjnldsv I meant if the solution you suggested works, you are welcome to add a commit :)

Well it works, I used the same as here nextcloud/mail#57 (review)

But I can't tell you if this will cause issue with the translators or not. I don't really knows how they work :)

@skjnldsv then go ahead and add a commit in this branch, you should be able to since you are in the Nextcloud organization ;) You know Javascript better than me. :)

Okay, I will commit. But we need to be sure the issue you raised with the translators is fine! :)

@skjnldsv Looks good 👍

skjnldsv · 2016-10-06T15:46:42Z

@jancborchardt Could we clarify the text error?
#1606 (comment)

skjnldsv · 2016-10-11T12:22:20Z

Commit pushed @jancborchardt ! :)

jancborchardt · 2016-10-11T12:56:38Z

@skjnldsv cool! Did that now also fix the wording which @eppfel suggested?

(btw @skjnldsv did you set up your email locally in git? Because it doesn’t show your avatar on the commit. :)

jancborchardt · 2016-10-11T12:57:35Z

core/search/js/search.js

@@ -217,7 +217,8 @@
 					$status.addClass('emptycontent').removeClass('status');
 					$status.html('');
 					$status.append('<div class="icon-search"></div>');
-					$status.append('<h2>' + t('core', 'No search results in other folders {filter}', {filter:lastQuery}) + '</h2>');
+					var error = t('core', "No search results in other folders '{tag}{filter}{endtag}'", {filter:lastQuery});


@skjnldsv »No search results in other folders for« so the wording which @eppfel remarked is fixed also :)

Pushed & rebased 👍

Fix wording

skjnldsv · 2016-10-11T13:11:19Z

@eppfel Review update pls? 😄

jancborchardt · 2016-10-18T08:53:22Z

@nickvergessen @MorrisJobke @eppfel another review please? ;)

MorrisJobke · 2016-10-18T11:47:22Z

👍

MorrisJobke · 2016-10-18T11:47:58Z

Counting @eppfel approval as 👍 ;) Merge it

search design details

a45cfbc

jancborchardt added enhancement design Design, UI, UX, etc. 3. to review Waiting for reviews labels Oct 1, 2016

jancborchardt added this to the Nextcloud 11.0 milestone Oct 1, 2016

eppfel requested changes Oct 1, 2016

View reviewed changes

Use the correct query variable

35a7a4f

Signed-off-by: Joas Schilling <coding@schilljs.com>

MorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Oct 5, 2016

skjnldsv reviewed Oct 6, 2016

View reviewed changes

jancborchardt commented Oct 11, 2016

View reviewed changes

Fix bold no results search text

1930aa3

Fix wording

skjnldsv force-pushed the search-detail branch from 4f07cba to 1930aa3 Compare October 11, 2016 13:06

MorrisJobke added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Oct 11, 2016

eppfel approved these changes Oct 18, 2016

View reviewed changes

MorrisJobke merged commit 2bb031e into master Oct 18, 2016

MorrisJobke deleted the search-detail branch October 18, 2016 11:48

marcelklehr mentioned this pull request Jun 1, 2023

fix share roots always being marked as writable #38179

Merged

search design details #1606

search design details #1606

Conversation

jancborchardt commented Oct 1, 2016

eppfel left a comment

Choose a reason for hiding this comment

eppfel Oct 1, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

te-online Oct 1, 2016 • edited Loading

Choose a reason for hiding this comment

te-online Oct 1, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skjnldsv Oct 5, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skjnldsv commented Oct 5, 2016 • edited Loading

nickvergessen commented Oct 5, 2016

skjnldsv commented Oct 5, 2016 • edited Loading

eppfel commented Oct 5, 2016

skjnldsv commented Oct 5, 2016

MorrisJobke commented Oct 5, 2016

MorrisJobke commented Oct 5, 2016

skjnldsv commented Oct 6, 2016

jancborchardt commented Oct 6, 2016

jancborchardt commented Oct 6, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LukasReschke Oct 6, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skjnldsv Oct 6, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skjnldsv commented Oct 6, 2016

skjnldsv commented Oct 11, 2016

jancborchardt commented Oct 11, 2016

Choose a reason for hiding this comment

skjnldsv Oct 11, 2016 • edited Loading

Choose a reason for hiding this comment

skjnldsv commented Oct 11, 2016

jancborchardt commented Oct 18, 2016

MorrisJobke commented Oct 18, 2016

MorrisJobke commented Oct 18, 2016

eppfel Oct 1, 2016 •

edited

Loading

te-online Oct 1, 2016 •

edited

Loading

te-online Oct 1, 2016 •

edited

Loading

skjnldsv Oct 5, 2016 •

edited

Loading

skjnldsv commented Oct 5, 2016 •

edited

Loading

skjnldsv commented Oct 5, 2016 •

edited

Loading

jancborchardt commented Oct 6, 2016 •

edited

Loading

LukasReschke Oct 6, 2016 •

edited

Loading

skjnldsv Oct 6, 2016 •

edited

Loading

skjnldsv Oct 11, 2016 •

edited

Loading