Closed Bug 886754 Opened 11 years ago Closed 10 years ago

[Contacts] No Search Highlight Apparent in Contact Search

Categories

(Firefox OS Graveyard :: Gaia::Contacts, defect, P1)

Product:
Firefox OS Graveyard
Component:
Gaia::Contacts
Platform:
All
Other
Type:
defect

Tracking

(feature-b2g:2.0, tracking-b2g:backlog, b2g-v2.0 fixed)

Status:
RESOLVED FIXED
Milestone:
2.0 S1 (9may)
Project Flags:
feature-b2g 2.0
tracking-b2g backlog
Tracking Flags:
Tracking Status
b2g-v2.0 --- fixed

People

(Reporter: epang, Assigned: mbudzynski)

References

Details

(Whiteboard: visual design, visual-tracking, bokken, [systemsfe], [priority])

Attachments

(2 files, 1 obsolete file)

Highlight Search - Contacts
92.32 KB, image/png
Details
Final Patch
46 bytes, text/x-github-pull-request
bkelly
: review+
Details | Review 
Pavel,

Can you please add search highlights for contact search?
This should function similarly to Email search.

The highlight should be:
#b2f2ff 100% Opacity

Thanks!
No longer blocks: 885691
Hey Eric,
I think I'll do it slowly. Maybe Sam can do this faster than me.
(In reply to Pavel Ivanov [:ivanovpavel] from comment #1)
> Hey Eric,
> I think I'll do it slowly. Maybe Sam can do this faster than me.

Hi Sam,
Can you help us add the search highlight for contact search?
Flags: needinfo?(sjochimek)
Hi guys,

I noticed this missing feature and added it while working on bug 866902.  The pull request is at https://github.com/mozshiao9/gaia/commit/e01b9a4b081ff7aa4b89df56ed83c0c282d852c3

The code hasn't been reviewed and is probably outdated since UX has been quite busy and unavailable to validate the visual/transition changes.  

Since bug 866902 is a WIP, I would like to know if I can take this and work on it. Given no major changes, my patch might still be applicable.

Thanks,
Mark
Pavel/Sam, where did we leave off on this?  Will the patch that Mark create help?
Flags: needinfo?(pivanov)
I think Sam can tell more than me here
Assignee: pivanov → nobody
Flags: needinfo?(pivanov)
Hey Sam, can you help implement this?
Assignee: nobody → sjochimek
Hi Pavel, I'm passing this bug back to you.  Can you help implement?  Let me know when it's ready and I can test and provide feedback.  Also when ready for review please flag to the component owner. Thanks!
Assignee: sjochimek → pivanov
Flags: needinfo?(sjochimek)
Attached file patch for Gaia/master (obsolete) — 
Hey Francisco,
can you check this one. I used the code from Mark Shiao
Attachment #815366 - Flags: review?(francisco.jordano)
Comment on attachment 815366 [details]
patch for Gaia/master

Hi Pivanov,

can you review the unit tests:

https://travis-ci.org/mozilla-b2g/gaia/jobs/12373265

I've tried to pass them twice and getting that error.

Thanks a lot for your work!
Attachment #815366 - Flags: review?(francisco.jordano) → review-
(In reply to Francisco Jordano [:arcturus] from comment #10)
> Comment on attachment 815366 [details]
> patch for Gaia/master
> 
> Hi Pivanov,
> 
> can you review the unit tests:
> 
> https://travis-ci.org/mozilla-b2g/gaia/jobs/12373265
> 
> I've tried to pass them twice and getting that error.
> 
> Thanks a lot for your work!

Hey Pavel, can you take a look at Francisco's Comment?  Thanks!
Flags: needinfo?(pivanov)
Hey Eric,
I fixed some of the issues but still have one and I write email to Francisco for suggestion and now waiting for answer
Flags: needinfo?(pivanov)
(In reply to Pavel Ivanov [:ivanovpavel] from comment #12)
> Hey Eric,
> I fixed some of the issues but still have one and I write email to Francisco
> for suggestion and now waiting for answer

ok sounds good! Thanks Pavel!
Whiteboard: visual design, visual-tracking → visual design, visual-tracking, jian
Note: I need a device for this one
Blocks: SysFE
Whiteboard: visual design, visual-tracking, jian → visual design, visual-tracking, bokken
Blocks: contacts-visual-refr
No longer blocks: SysFE
Depends on: 944418
No longer depends on: 944418
blocking-b2g: --- → 1.4?
Not a blocker please land it once it is ready.
blocking-b2g: 1.4? → ---
Hey Sam,

Did you have a chance to look at this one?
Flags: needinfo?(sjochimek)
Assignee: pivanov → sjochimek
Flags: needinfo?(sjochimek)
Whiteboard: visual design, visual-tracking, bokken → visual design, visual-tracking, bokken, [systemsfe]
Target Milestone: --- → 1.4 S3 (14mar)
blocking-b2g: --- → backlog
Whiteboard: visual design, visual-tracking, bokken, [systemsfe] → visual design, visual-tracking, bokken, [systemsfe], [priority]
Target Milestone: 1.4 S3 (14mar) → 1.4 S4 (28mar)
Target Milestone: 1.4 S4 (28mar) → ---
ni? Sam
Hi Sam, i wonder if this bug is still worked on. Thanks
Flags: needinfo?(sjochimek)
I'll be happy to take it.
Assignee: sjochimek → mbudzynski
Target Milestone: --- → 2.0 S1 (9may)
I've work on this bug in branch 'Bug886754' in my repo [1]. In the first WIP patch I catch all the nodes that we are interested in (when searching will go thru all the contacts) and passing them to the 'highlightNode' function [2]. Working on this function now.

[1] https://github.com/michalbe/gaia/tree/Bug886754
[2] https://github.com/michalbe/gaia/commit/6a87d28e7fdb4e63db01ac23898c2948af612ca1
I've created a Regular Expression that match searched phrase in the new WIP patch [1]. 

[1] https://github.com/michalbe/gaia/commit/0c3aef4a5d7594e599fdbfee8b22daa0c1511f1b#diff-d965a5a85955e09a269c4a64142c0956R402
Previous patch worked only on first letter because when regex was trying to match the second one, the previous was already wrapped in highlighting spans. This commit solves this problem [1].

[1] https://github.com/michalbe/gaia/commit/9b335ca40931e69f1a800a9f9c22054147e1fc63
Because of the noticeable delay of highlighting on a huge contacts list, the next patch [1] operates on single nodes while adding them to the list, not on a whole list. The delay doesn't occur anymore.

[1] https://github.com/michalbe/gaia/commit/055a6afa8968bc19441c6b3ff025249ec87eb397
Next commit [1] fixes multiple letters highlight.

[1] https://github.com/michalbe/gaia/commit/927e7083b99127ff57d2f573a76b361080ec1520
Because on the initial search only one chunk of contacts is displayed, highlighting needs to be executed also when we display remaining results [1].

[1] https://github.com/michalbe/gaia/commit/e075c84edad096d2f0b06fda3edf659754936bc3
Attached file Final Patch — 

        Attachment #815366 -
        Attachment is obsolete: true

        Attachment #8414418 -
        Flags: review?(bkelly)

    
    

    
  
Final patch ready with test. Waiting for r? from :bkelly
Flags: needinfo?(sjochimek) → needinfo?(bkelly)

    
    

    
  
Sorry, I was out a bit today.  I will most likely review this tomorrow morning (UTC-4).
Flags: needinfo?(bkelly)

    
    

    
  
Great, thanks.

    
    

    
  
Comment on attachment 8414418 [details] [review]
Final Patch

Looks good!  I just requested a few explanatory comments on github.  I verified that it works well on my buri with reference-workload-heavy.  This also adds tests and travis is green.

Well done! r=me

        Attachment #8414418 -
        Flags: review?(bkelly) → review+

    
    

    
  
Thank you for your r Ben! I've updated the comments, Travis is green, patch landed in master [1]. 

[1] https://github.com/mozilla-b2g/gaia/commit/8f40d7e84b3070807259329db0112e812b00d681
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
feature-b2g: --- → 2.0
blocking-b2g: backlog → ---

          tracking-b2g:
          --- → backlog

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: