1171883 - [Statusbar] Signal icon is not animated while searching

Status: RESOLVED FIXED

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

Description

[Alberto Pastor [:albertopq]]

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

Comment 1

[Alberto Pastor [:albertopq]]

Regression of bug 1151502. :gmarty, could you please take a look?

Comment 2

[Guillaume Marty [:gmarty]]

Attachment: Github PR

I reworked the logic for the searching state.
What do you think, Alberto?

Comment 3

[Alberto Pastor [:albertopq]]

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

Comment 4

[Etienne Segonzac (: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

Comment 5

[Guillaume Marty [:gmarty]]

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.

Comment 6

[Alberto Pastor [:albertopq]]

Attachment: no-sginal-icon.png

I can't see any icon now, while searching

Comment 7

[Alberto Pastor [:albertopq]]

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!

Comment 8

[Guillaume Marty [:gmarty]]

Not at all. Go for it!

Comment 9

[Alberto Pastor [:albertopq]]

Attachment: Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/30646

Comment 10

[Guillaume Marty [: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.

Comment 11

[Alberto Pastor [:albertopq]]

master: https://github.com/mozilla-b2g/gaia/commit/4d7da51b3923a0f54748c6619919e59c91058bb5