Closed Bug 961572 Opened 6 years ago Closed 5 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(b2g-master fixed)

RESOLVED FIXED
2.2 S5 (6feb)
Tracking Status
b2g-master --- fixed

People

(Reporter: steveck, Assigned: ythej, Mentored)

References

Details

(Whiteboard: [lang=css][lang=js][sms-most-wanted])

Attachments

(1 file)

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.
Even a complete header ;)
Assignee: schung → nobody
Mentor: schung
Whiteboard: [lang=css][lang=js]
Summary: [Messages] Implement a new independent cancel button for information view's header → [Messages][Refactoring] Implement a new independent cancel button for information view's header
Whiteboard: [lang=css][lang=js] → [lang=css][lang=js][sms-most-wanted]
Assignee: nobody → yvtheja
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?
Flags: needinfo?(schung)
Hi Steve,
got the answer in IRC. :)
Thank you :)
Flags: needinfo?(schung)
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 :)
Attachment #8550896 - Flags: review?(schung)
Hi Julien,
May I file a new bug for transition of group view panel?
Flags: needinfo?(felash)
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.
Attachment #8550896 - Flags: review?(schung) → feedback?(schung)
(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.
Attachment #8550896 - Flags: feedback?(schung) → review?(schung)
Hi Steve,
I did some change in information_test.js file.I don't know if we need extra unit tests.
Thank you :)
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 !
Flags: needinfo?(felash)
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?).
Attachment #8550896 - Flags: review?(schung)
Hi Steve,
I got it! I may be late for submitting the patch as I am not okay with my health.  
Thank you :)
File a new bug for transition of group view -> bug 1124624
Comment on attachment 8550896 [details] [review]
pull request for the bug 961572

Hi Steve,
I covered Integration tests.
Thank you.
Attachment #8550896 - Flags: review?(schung)
(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 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!
Attachment #8550896 - Flags: review?(schung) → review+
Keywords: checkin-needed
Master: https://github.com/mozilla-b2g/gaia/commit/8f9a37096d430528393215f122ae59ddf2c61828
Status: NEW → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S5 (6feb)
You need to log in before you can comment on or make changes to this bug.