Closed
Bug 961572
Opened 11 years ago
Closed 10 years ago
[Messages][Refactoring] Implement a new independent cancel button for information view's header
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect)
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.
Comment 1•11 years ago
|
||
Even a complete header ;)
Updated•10 years ago
|
Assignee: schung → nobody
Mentor: schung
Whiteboard: [lang=css][lang=js]
Updated•10 years ago
|
Blocks: sms-refactoring
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]
Updated•10 years ago
|
Assignee: nobody → yvtheja
Assignee | ||
Comment 2•10 years ago
|
||
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)
Assignee | ||
Comment 3•10 years ago
|
||
Hi Steve,
got the answer in IRC. :)
Thank you :)
Flags: needinfo?(schung)
Assignee | ||
Comment 4•10 years ago
|
||
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)
Assignee | ||
Comment 5•10 years ago
|
||
Hi Julien,
May I file a new bug for transition of group view panel?
Flags: needinfo?(felash)
Reporter | ||
Comment 6•10 years ago
|
||
Comment on attachment 8550896 [details]
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)
Reporter | ||
Comment 7•10 years ago
|
||
(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.
Assignee | ||
Updated•10 years ago
|
Attachment #8550896 -
Flags: feedback?(schung) → review?(schung)
Assignee | ||
Comment 8•10 years ago
|
||
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•10 years ago
|
||
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)
Reporter | ||
Comment 10•10 years ago
|
||
Comment on attachment 8550896 [details]
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)
Assignee | ||
Comment 11•10 years ago
|
||
Hi Steve,
I got it! I may be late for submitting the patch as I am not okay with my health.
Thank you :)
Assignee | ||
Comment 12•10 years ago
|
||
File a new bug for transition of group view -> bug 1124624
Assignee | ||
Comment 13•10 years ago
|
||
Comment on attachment 8550896 [details]
pull request for the bug 961572
Hi Steve,
I covered Integration tests.
Thank you.
Attachment #8550896 -
Flags: review?(schung)
Reporter | ||
Comment 14•10 years ago
|
||
(In reply to Vishnu Teja [:ythej] from comment #13)
> Comment on attachment 8550896 [details]
> 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.
Reporter | ||
Comment 15•10 years ago
|
||
Comment on attachment 8550896 [details]
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+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 16•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
status-b2g-master:
--- → fixed
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.
Description
•