Francesco Lodolo [:flod] Reporter
	Description • 10 years ago

For more than 10 matches, header is displayed as

> {{ n }} matches for "123"

This line sets the number to "10+"
https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/suggestion_bar.js#L129

This one from bug 1022445 converts the string to a number, but fails for "10+"
https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/suggestion_bar.js#L331

	Francesco Lodolo [:flod] Reporter
	Comment 1 • 10 years ago

Some doubts also about the meaning of "10+": as it's currently implemented, it means "10 or more", personally I'd expect it to mean "more than 10".

	Jason Smith [:jsmith]
	Comment 2 • 10 years ago

QA Wanted for branch checks.

	Francesco Lodolo [:flod] Reporter
	Comment 3 • 10 years ago

Actually, the problem is worse: assuming that there are 15 matches, textContent says "10+". 
If you fix the conversion problem, for example using parseInt, you end up with an header saying "10 matches", which is not true.

	Joshua Mitchell (Inactive)
	Comment 4 • 10 years ago

(In reply to Jason Smith [:jsmith] from comment #2)
> QA Wanted for branch checks.

This issue Repro's on Flame 2.1, Flame 2.0, and Buri 2.1

Environmental Variables:
Device: Flame Master
Build ID: 20140707060819
Gaia: 99f56d9db3cd37c684b01de6fed786421f47e2b7
Gecko: 085eea991bb9
Version: 33.0a1 (Master)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0

Environmental Variables:
Device: Flame 2.0
Build ID: 20140707101630
Gaia: 16410831c2e513b5cabc550363f28a023dc256be
Gecko: 05069bac98d8
Version: 32.0a2 (2.0)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.

Environmental Variables:
Device: Buri Master
Build ID: 20140707060819
Gaia: 99f56d9db3cd37c684b01de6fed786421f47e2b7
Gecko: 085eea991bb9
Version: 33.0a1 (Master)
Firmware Version: v1.2device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0

------------------------------------------------------------------------------------
Issue does NOT repro on Flame 1.4

Actual Results: Header shows "10+ matches for "555"

Environmental Variables:
Device: Flame 1.4
Build ID: 20140707000200
Gaia: 5c9e1e4131d3ac8915ed88b72bb66dc7d97be6a0
Gecko: 2d0c15450488
Version: 30.0 (1.4)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

	Francesco Lodolo [:flod] Reporter
	Comment 5 • 10 years ago

@Jason, this is caused by the bug I set as a dependency (bug 1022445), no need to find a regression window.

	Francesco Lodolo [:flod] Reporter
	Comment 6 • 10 years ago

We were also discussing this with :stas on IRC, and the form "10+" has some issues too:
* It's not necessarily common or easy to understand in all cultures.
* It's prone to confusion: does it include 10 or not? Looking at the code it does, but it's not evident, at least for me.

Given that we don't display suggestions before user enters at least 3 numbers, I wonder if we could just display the actual number of matches, I doubt they will reach 3 digits.

	Jason Smith [:jsmith]
	Comment 7 • 10 years ago

QA Wanted - Need a screenshot of the bug here.

	Joshua Mitchell (Inactive)
	Comment 9 • 10 years ago

Screenshot added

	Staś Małolepszy :stas
	Comment 10 • 10 years ago

(In reply to Francesco Lodolo [:flod] from comment #6)
> Given that we don't display suggestions before user enters at least 3
> numbers, I wonder if we could just display the actual number of matches, I
> doubt they will reach 3 digits.

This sounds like the easiest solution.  Looping in the UX team.

	Carrie Wang [New account: carrie75521@gmail.com]
	Comment 11 • 10 years ago

Hi, 
I didn't define that we should list "10+" when there are more than 10 matched results.I think this is provided from visual side(?), but yes, I agree with you, we should display the real number unless it's three digit numbers (we display 99+ for this edge case). What do you think? Thanks!

	Wesley Huang [:wesley_huang] (EPM) (NI me)
	Comment 12 • 10 years ago

triage: obvious flaw in user experience. 2.0+
ni? Vicky to feedback on comment 11.

	Wesley Huang [:wesley_huang] (EPM) (NI me)
	Comment 13 • 10 years ago

Be noted that the list only shows up to 10 numbers (refer to the screenshot https://bugzilla.mozilla.org/attachment.cgi?id=8452481).
It looks like a current design, so I doubt if displaying "real number" make the list look weird.

	Anthony Ricaud (:rik)
	Comment 14 • 10 years ago

We can also change the algorithm to not propose a suggestion if you have more than X matches. Because if you have 50 people matching a number, you're probably gonna type one more digit to filter that down anyway.

	Victoria Gerchinhoren [:vicky]
	Comment 15 • 10 years ago

(In reply to Carrie Wang [:carrie] from comment #11)
> Hi, 
> I didn't define that we should list "10+" when there are more than 10
> matched results.I think this is provided from visual side(?), but yes, I
> agree with you, we should display the real number unless it's three digit
> numbers (we display 99+ for this edge case). What do you think? Thanks!

Hey, no, this is not something proposed from the visual side at all, I think it is more of an implementation decision. Visually there's no document that can define this type of specification, it always comes from an IA or implementation decision.

Cheers,
V

	Anthony Ricaud (:rik)
	Comment 16 • 10 years ago

Thanks Vicky.

Carrie: What do you think about my proposition in comment 14? If we do that, that means we only display the suggestion bar when we have more than 3 digits and less than X matches.

	Carrie Wang [New account: carrie75521@gmail.com]
	Comment 17 • 10 years ago

Yes, let's take 50 as our magic number. Thanks!

	Wesley Huang [:wesley_huang] (EPM) (NI me)
	Comment 18 • 10 years ago

Thank you Carrie for the clarification. My understanding of the expected behavior is as below:

1. User types 3 (or more) digits, and the suggestion display shows once the match has less than 50 contacts. 
2. It shows with the actual number instead of something like "10+". 
3. User can tap on the actual number to go to the screen with all the matches listed. Scrollable, and up to 50.

	Doug Sherk (:drs) (inactive) Assignee
	Comment 20 • 10 years ago

PR: https://github.com/mozilla-b2g/gaia/pull/21778

	Anthony Ricaud (:rik)
	Comment 21 • 10 years ago

Comment on attachment 8456380 [details] [diff] [review]
Make matched suggestions not appear when there are more than 50, remove + indicator.

Review of attachment 8456380 [details] [diff] [review]:
-----------------------------------------------------------------

Nice to have 3 different new tests! We're still missing at least one to check that we are displaying all of them in Suggestion List. This will test your +1 / >= changes. Once you have this test, I think we may be able to remove some of the .slice() that are now useless because we always display all of the results.

	Doug Sherk (:drs) (inactive) Assignee
	Comment 22 • 10 years ago

Updated PR.

	Anthony Ricaud (:rik)
	Comment 23 • 10 years ago

Comment on attachment 8456952 [details] [diff] [review]
Make matched suggestions not appear when there are more than 50, remove + indicator.

Review of attachment 8456952 [details] [diff] [review]:
-----------------------------------------------------------------

Only a +6 lines fix! I love fixing bugs with such low impact.

Ship it!

	Doug Sherk (:drs) (inactive) Assignee
	Comment 24 • 10 years ago

https://github.com/mozilla-b2g/gaia/commit/e17b3c93d31001643ac8cff2531e0fa1edab69d8

	Ryan VanderMeulen [:RyanVM]
	Comment 25 • 10 years ago

v2.0: https://github.com/mozilla-b2g/gaia/commit/c30159ff811d153373a783459fefb4efcb8a80c5

	Doug Sherk (:drs) (inactive) Assignee
	Comment 26 • 10 years ago

backed out on 2.0 due to test failures (thanks Germán for noticing):
https://github.com/mozilla-b2g/gaia/commit/9977a02ea62ba96425a1cc4d1bfb54a909f68905

I'll fix this and reland it.

	Doug Sherk (:drs) (inactive) Assignee
	Comment 27 • 10 years ago

Will request uplift of bug 1018494 to get the tests passing. We have a workaround but I'd prefer to just do the uplift instead.

	Doug Sherk (:drs) (inactive) Assignee
	Comment 28 • 10 years ago

v2.0: https://github.com/mozilla-b2g/gaia/commit/01382a55939b41067e780c61b46e4390576b7f09