We should split up the phone_lock.js file so the code for the dialog is separate from the main phone lock panel.
Attachment #831164 - Attachment description: WIP - Github pull request → Github pull request
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.
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?
(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 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?
Created attachment 8335125 [details] [review] 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!
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 on attachment 8335125 [details] [review] Alternative pull request Evelyn, any ideas?
Comment on attachment 8335125 [details] [review] Alternative pull request Seems like we're going with a different approach now.
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.