1111466 - [Stingray]Unpin card from context menu of app deck

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Users are able to unpin card from context menu of home app.

Comment 1

[Tzu-Lin Huang [:dwi2][:tzhuang]]

to be able to unpin card from

Comment 2

[Tzu-Lin Huang [:dwi2][:tzhuang]]

comment 0 is incorrect. This bug should be - Unpin card from context menu of app deck.
First, we should let app deck know the whole card list (fetch from datastore)

Comment 3

[Tzu-Lin Huang [:dwi2][:tzhuang]]

WIP https://github.com/dwi2/gaia/tree/bug1111466

DONE:
1. Move logic of `serialize` and `deserialize` card into card-like classes
2. Move card_store, piped_promise, and card-like classes into tv_shared
3. Implement card_reader for app-deck

TODO:
1. Because there are a lot of common in card_reader and card_manager (of smart-home), I'll drop card_reader and reuse card_manager as much as possible. I'll move card_manager to tv_shared and make it dual-mode (read mode for app-deck, and write mode for smart-home)
2. Unpin logic

Comment 4

[Tzu-Lin Huang [:dwi2][:tzhuang]]

status report:
DONE:
1. Move logic of `serialize` and `deserialize` card into card-like classes
2. Move card_store, piped_promise, and card-like classes into tv_shared
3. Implement card_reader for app-deck
4. Because there are a lot of common in card_reader and card_manager (of smart-home), I'll drop card_reader and reuse card_manager as much as possible. I'll move card_manager to tv_shared and make it dual-mode (read mode for app-deck, and write mode for smart-home)
5. Unpin logic

TODO:
1. there are some state transition error in card_manager. I am still looking at it.

Comment 5

[Tzu-Lin Huang [:dwi2][:tzhuang]]

status report:
I am almost done. Only need to remove my debugging console.log() and make it more reviewing-friendly. I guess I could send review request tomorrow.

Comment 6

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Attachment: pull request

Hi John,

Since Rex is out-of-office, please help to review the patch. Thanks

We did the following in this patch:

    1. Move logic of serialize and deserialize into card-like classes
    2. Move CardStore, PipedPromise, and card-like classes into tv_shared folder
    3. Implement dual mode of CardManager for App-Deck. Only Smart-Home can run CardManager in readwrite mode. All other apps can only run CardManager in readonly mode. CardManager now also have two states - READY and SYNCING. When CardManager are reading data from or writing data to CardStore, it is in SYNCING state.
    4. Add unpin logic in App-Deck and sending unpin activity to Smart-Home.

Comment 7

[John Hu [:johnhu][:johu][:醬糊小弟]]

Comment on attachment 8541571 [details] [review]
pull request

This patch looks good. But it may be better. Please find my comments in PR.

Comment 8

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Comment on attachment 8541571 [details] [review]
pull request

Hi John,
Your comments are addressed. But since you are also on PTO for a long period, I'll let Luke review the patch. I also flag you feedback? if you have time.

Hi Luke,

Would you help to review the patch? Thanks

Comment 9

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8541571 [details] [review]
pull request

Hi Tzu-Lin,

I left some comments on GitHub. Please take a look? Thanks.

Comment 10

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Comment on attachment 8541571 [details] [review]
pull request

Hi Luke,

Comment addressed. Please take a look at the patch again. Thanks

Comment 11

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8541571 [details] [review]
pull request

Looks great! Thanks.

Comment 12

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Thanks,

landed on master 
https://github.com/mozilla-b2g/gaia/commit/a2060fbf5cfae3f2d0bdaf95b248fb03ac93ec78