961572 - [Messages][Refactoring] Implement a new independent cancel button for information view's header

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::SMS, defect)

Description

[Steve Chung [:steveck]]

It's an issue related to button hover/active visual effect in Bug #950623. Reusing same back button page switching might cause the button active effect fails to display correct style. We need to have another new button for information view to avoid other possible button styling bug.

Comment 1

[Julien Wajsberg [:julienw]]

Even a complete header ;)

Comment 2

[Vishnu Teja [:ythej]]

Hi Steve,
for implementing new header, do we need to create a new section below the 'thread-messages' for the article's containing view's and add new header in that?

Comment 3

[Vishnu Teja [:ythej]]

Hi Steve,
got the answer in IRC. :)
Thank you :)

Comment 4

[Vishnu Teja [:ythej]]

Attachment: pull request for the bug 961572

Hi Steve,
I wanted know if I miss any case while implementing new sections for both the view's and if all functions are declared at right place before going to unit tests.
Thank you :)

Comment 5

[Vishnu Teja [:ythej]]

Hi Julien,
May I file a new bug for transition of group view panel?

Comment 6

[Steve Chung [:steveck]]

Comment on attachment 8550896 [details] [review]
pull request for the bug 961572

Please remeber to check the tests because your patch involves js changes. In this patch you definitely breaks the unit test in information.js(maybe threaui as well). You can use feedback flag instead if you could not finish the test part at first. You can request review once the everything is ready.

Comment 7

[Steve Chung [:steveck]]

(In reply to Vishnu Teja from comment #5)
> Hi Julien,
> May I file a new bug for transition of group view panel?

You can fire a new bug for sure, and please ni? UX about the animation. Not sure if sliding transistion is still needed for information panel.

Comment 8

[Vishnu Teja [:ythej]]

Hi Steve,
I did some change in information_test.js file.I don't know if we need extra unit tests.
Thank you :)

Comment 9

[Julien Wajsberg [:julienw]]

We need unit test for every piece of code you change :)

(In reply to Vishnu Teja from comment #5)
> Hi Julien,
> May I file a new bug for transition of group view panel?

Please do !

Comment 10

[Steve Chung [:steveck]]

Comment on attachment 8550896 [details] [review]
pull request for the bug 961572

Except the failed test in integration test, I also left some comments on github. Please note that Information is prototype for group view and report view, so you will need to define the header and text in View. Having reportHeader/groupHeader in Information doesn't make sense(It's weird to see groupView.reportHeader existed, right?).

Comment 11

[Vishnu Teja [:ythej]]

Hi Steve,
I got it! I may be late for submitting the patch as I am not okay with my health.  
Thank you :)

Comment 12

[Vishnu Teja [:ythej]]

File a new bug for transition of group view -> bug 1124624

Comment 13

[Vishnu Teja [:ythej]]

Comment on attachment 8550896 [details] [review]
pull request for the bug 961572

Hi Steve,
I covered Integration tests.
Thank you.

Comment 14

[Steve Chung [:steveck]]

(In reply to Vishnu Teja [:ythej] from comment #13)
> Comment on attachment 8550896 [details] [review]
> pull request for the bug 961572
> 
> Hi Steve,
> I covered Integration tests.
> Thank you.

I gave some comments on github, but the overall looks good now.

Comment 15

[Steve Chung [:steveck]]

Comment on attachment 8550896 [details] [review]
pull request for the bug 961572

Only one nit, please use checkin-needed once you finished and thanks for the great job!

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/8f9a37096d430528393215f122ae59ddf2c61828