937768 - [Settings] Split up phone_lock.js

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia, defect)

Description

[Kevin Grandon :kgrandon]

We should split up the phone_lock.js file so the code for the dialog is separate from the main phone lock panel.

Comment 1

[Kevin Grandon :kgrandon]

Attachment: Github pull request

Comment 2

[Kevin Grandon :kgrandon]

Comment on attachment 831164 [details] [review]
Github pull request

Hey Arthur - could you tell me what you think of this one?

I needed a way to pass arguments to panels, and for now data attributes seem to be a fairly sane way to do this. In the future I think I'd like to move to that proposal I emailed you about - but this is probably the best option with the least amount of code changes.

In order to pass arguments around, I changed the currentPanel setter to a changePanel() method which takes a panel id and params object. Right now it's only using dataset on the panel section, but we can change this in the future to be a URL, or object instantiation for example.

Comment 3

[Arthur Chen [:arthurcc][inactive after 6/30]]

Yes, I think we do need a way to pass arguments when changing panels. In addition to a params object, what do you think to have a callback function as the third parameter, so that the navigated panel can bring the results back?

Comment 4

[Kevin Grandon :kgrandon]

(In reply to Arthur Chen [:arthurcc] from comment #3)
> Yes, I think we do need a way to pass arguments when changing panels. In
> addition to a params object, what do you think to have a callback function
> as the third parameter, so that the navigated panel can bring the results
> back?

I would really rather avoid yet another callback from panel startup. I think passing the arguments into the panel during instantiation is the way to go.

Comment 5

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 831164 [details] [review]
Github pull request

Hi Kevin, sorry for the late review. 

I left some comments regarding the part of passing data to panels. wdyt?

Comment 6

[Kevin Grandon :kgrandon]

Attachment: Alternative pull request

Hi Arthur,

I agree that using data attributes for passing data to panels is rather fragile. I would much rather have something like this which has a defined interface for passing data into a panel. In this case the render method is called whenever the panel is shown, and it can take any necessary data passed into it.

This would be a migration that I would start doing for other panels, and the upcoming work to remove sub-panel dependencies.

I still need to do more testing, but wanted to get some initial feedback on this approach. Thanks!

Comment 7

[Kevin Grandon :kgrandon]

Note: Here's some important bits from the PR:

Defining a panel object: https://github.com/mozilla-b2g/gaia/pull/13861/files#diff-f27ad564aa7339154e5f506da70a0f6cR244

Render method being called on cached panel objects: https://github.com/mozilla-b2g/gaia/pull/13861/files#diff-f36e65bf103d8ff227c8349e0411b107R69

Data being utilized in the render method: https://github.com/mozilla-b2g/gaia/pull/13861/files#diff-f27ad564aa7339154e5f506da70a0f6cR68

Comment 8

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8335125 [details] [review]
Alternative pull request

Evelyn, any ideas?

Comment 9

[Kevin Grandon :kgrandon]

Comment on attachment 8335125 [details] [review]
Alternative pull request

Seems like we're going with a different approach now.