Closed Bug 1171883 Opened 7 years ago Closed 6 years ago

[Statusbar] Signal icon is not animated while searching

Categories

(Firefox OS Graveyard :: Gaia::System::Status bar, Utility tray, Notification, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
2.2 S14 (12june)

People

(Reporter: apastor, Assigned: apastor)

References

Details

(Keywords: regression, Whiteboard: [systemsfe])

Attachments

(2 files, 1 obsolete file)

STR:

1.- Start the phone and insert the PIN

Expected:

The signal icon gets animated while searching signal. Then it stops with as many bars as the signal offers.

Actual:

The signal icon is static with 0 bars
Regression of bug 1151502. :gmarty, could you please take a look?
Flags: needinfo?(gmarty)
Keywords: regression
Assignee: nobody → gmarty
Target Milestone: --- → 2.2 S14 (12june)
Blocks: 1151502
Attached file Github PR (obsolete) —
I reworked the logic for the searching state.
What do you think, Alberto?
Flags: needinfo?(gmarty)
Attachment #8620928 - Flags: review?(apastor)
Mhm, I'm not completely sure about the solution here. In my opinion, the code was right before bug 1151502 landed. In theory, we should receive an 'update' event once is connected, and that event already calls updateSignal. If that's not updating the icon correctly, we need to investigate why, but adding more conditions to the updateSignal method doesn't seem correct to me.

The other option is moving all the 'searching' logic inside the updateSignal method (which I think is your idea), but then I would reestructurate the 'update' method [0]. I wouldn't set the 'searching' dataset there, then [1], for example.

Asking for a second opinion. Etienne?

[0] https://github.com/gmarty/gaia/blob/Bug-1171883-Signal-icon-is-not-animated-while-searching/apps/system/js/signal_icon.js#L28

[1] https://github.com/gmarty/gaia/blob/Bug-1171883-Signal-icon-is-not-animated-while-searching/apps/system/js/signal_icon.js#L84
Flags: needinfo?(etienne)
(In reply to Alberto Pastor [:albertopq] from comment #3)
> Mhm, I'm not completely sure about the solution here. In my opinion, the
> code was right before bug 1151502 landed. In theory, we should receive an
> 'update' event once is connected, and that event already calls updateSignal.
> If that's not updating the icon correctly, we need to investigate why, but
> adding more conditions to the updateSignal method doesn't seem correct to me.
> 
> The other option is moving all the 'searching' logic inside the updateSignal
> method (which I think is your idea), but then I would reestructurate the
> 'update' method [0]. I wouldn't set the 'searching' dataset there, then [1],
> for example.
> 
> Asking for a second opinion. Etienne?

Correct me if I'm wrong but looks like [0] is the key change in Guillaume's PR.
If so, I agree with both of you I think, let's move [1] to updateSignal and go whith that :)


[0] https://github.com/mozilla-b2g/gaia/pull/30501/files?w=1#diff-ea8d19ab311e1f3ec83ba5d1df0fccd0R114
[1] https://github.com/gmarty/gaia/blob/Bug-1171883-Signal-icon-is-not-animated-while-searching/apps/system/js/signal_icon.js#L84-86
Flags: needinfo?(etienne)
I updated the patch according to Etienne's comment.
Alberto, can I ask you to try? I can't reproduce this bug because my SIM card connects too quickly to the network.
Attached image no-sginal-icon.png
I can't see any icon now, while searching
Hey Guillaume, is probably going to be easier if I just steal this bug, because I'll be able to test it. Do you mind if I take this one with your WIP patch?

Thanks!
Flags: needinfo?(gmarty)
Not at all. Go for it!
Assignee: gmarty → apastor
Flags: needinfo?(gmarty)
Attachment #8620928 - Attachment is obsolete: true
Attachment #8620928 - Flags: review?(apastor)
Attachment #8624143 - Flags: review?(gmarty)
Comment on attachment 8624143 [details] [review]
Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/30646

It looks good. Thanks for taking over me.
Attachment #8624143 - Flags: review?(gmarty) → review+
master: https://github.com/mozilla-b2g/gaia/commit/4d7da51b3923a0f54748c6619919e59c91058bb5
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.