{"action":"created","issue":{"url":"https://api.github.com/repos/uProxy/uproxy/issues/2755","repository_url":"https://api.github.com/repos/uProxy/uproxy","labels_url":"https://api.github.com/repos/uProxy/uproxy/issues/2755/labels{/name}","comments_url":"https://api.github.com/repos/uProxy/uproxy/issues/2755/comments","events_url":"https://api.github.com/repos/uProxy/uproxy/issues/2755/events","html_url":"https://github.com/uProxy/uproxy/pull/2755","id":175833660,"number":2755,"title":"Hotpot prototype","user":{"login":"fortuna","id":113565,"avatar_url":"https://avatars.githubusercontent.com/u/113565?v=3","gravatar_id":"","url":"https://api.github.com/users/fortuna","html_url":"https://github.com/fortuna","followers_url":"https://api.github.com/users/fortuna/followers","following_url":"https://api.github.com/users/fortuna/following{/other_user}","gists_url":"https://api.github.com/users/fortuna/gists{/gist_id}","starred_url":"https://api.github.com/users/fortuna/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/fortuna/subscriptions","organizations_url":"https://api.github.com/users/fortuna/orgs","repos_url":"https://api.github.com/users/fortuna/repos","events_url":"https://api.github.com/users/fortuna/events{/privacy}","received_events_url":"https://api.github.com/users/fortuna/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/uProxy/uproxy/milestones/20","html_url":"https://github.com/uProxy/uproxy/milestones/HotPot","labels_url":"https://api.github.com/repos/uProxy/uproxy/milestones/20/labels","id":1939568,"number":20,"title":"HotPot","description":"Tasks for implementing the new HotPot User Experience.\r\n\r\nLatest mocks: https://folio.googleplex.com/uproxydesign/HotPot/014\r\n\r\nDesktop Prototype: https://flow.googleplex.com/viewer/171520001/1\r\nMobile Prototype: https://flow.googleplex.com/viewer/168610002/0\r\n\r\nOriginal Slides: https://docs.google.com/presentation/d/1hC8a6B0gRnQ3-XZmgnTMgl9baZVj92y2Jhfz_9lKonY/edit\r\n","creator":{"login":"fortuna","id":113565,"avatar_url":"https://avatars.githubusercontent.com/u/113565?v=3","gravatar_id":"","url":"https://api.github.com/users/fortuna","html_url":"https://github.com/fortuna","followers_url":"https://api.github.com/users/fortuna/followers","following_url":"https://api.github.com/users/fortuna/following{/other_user}","gists_url":"https://api.github.com/users/fortuna/gists{/gist_id}","starred_url":"https://api.github.com/users/fortuna/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/fortuna/subscriptions","organizations_url":"https://api.github.com/users/fortuna/orgs","repos_url":"https://api.github.com/users/fortuna/repos","events_url":"https://api.github.com/users/fortuna/events{/privacy}","received_events_url":"https://api.github.com/users/fortuna/received_events","type":"User","site_admin":false},"open_issues":8,"closed_issues":0,"state":"open","created_at":"2016-08-12T19:03:01Z","updated_at":"2016-09-08T18:53:36Z","due_on":null,"closed_at":null},"comments":1,"created_at":"2016-09-08T18:53:36Z","updated_at":"2016-09-08T20:28:10Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/uProxy/uproxy/pulls/2755","html_url":"https://github.com/uProxy/uproxy/pull/2755","diff_url":"https://github.com/uProxy/uproxy/pull/2755.diff","patch_url":"https://github.com/uProxy/uproxy/pull/2755.patch"},"body":"Hello team,\r\n\r\nThis is a demo for changes I'm making on how we write UI components. I'm looking for feedback.\r\n\r\nThe basic idea is that the UI components are completely dumb, and do not rely on any globals. Everything is injected by the main logic.\r\n\r\nThis allows us to actually reuse UI components in different applications, including simple demos for development.\r\n\r\nThe demo is at `hotpot_demo.html`. It's currently just an ugly UI showing the initial flow for setting up language and metrics sharing, but it shows that we can get the behavior we need using this new model.\r\n\r\nTo see it in action, run\r\n\r\n    python -m SimpleHTTPServer\r\n\r\nThen open http://localhost:8000/src/generic_ui/hotpot_demo.html\r\n\r\nPlease let me know what you think.\r\nNext steps are to integrate this idea in our actual binaries and then make it look good.\n\n<!-- Reviewable:start -->\n---\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/uproxy/uproxy/2755)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/uProxy/uproxy/issues/comments/245729500","html_url":"https://github.com/uProxy/uproxy/pull/2755#issuecomment-245729500","issue_url":"https://api.github.com/repos/uProxy/uproxy/issues/2755","id":245729500,"user":{"login":"dborkan","id":6494307,"avatar_url":"https://avatars.githubusercontent.com/u/6494307?v=3","gravatar_id":"","url":"https://api.github.com/users/dborkan","html_url":"https://github.com/dborkan","followers_url":"https://api.github.com/users/dborkan/followers","following_url":"https://api.github.com/users/dborkan/following{/other_user}","gists_url":"https://api.github.com/users/dborkan/gists{/gist_id}","starred_url":"https://api.github.com/users/dborkan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dborkan/subscriptions","organizations_url":"https://api.github.com/users/dborkan/orgs","repos_url":"https://api.github.com/users/dborkan/repos","events_url":"https://api.github.com/users/dborkan/events{/privacy}","received_events_url":"https://api.github.com/users/dborkan/received_events","type":"User","site_admin":false},"created_at":"2016-09-08T20:28:10Z","updated_at":"2016-09-08T20:28:10Z","body":"Cool!  I'd also check with @lucyhe and @jpevarnek on their thoughts on this design.\n\n---\n\nReview status: 0 of 5 files reviewed at latest revision, 6 unresolved discussions.\n\n---\n\n*[src/generic_ui/hotpot_demo.html, line 40 \\[r1\\]](https://reviewable.io:443/reviews/uproxy/uproxy/2755#-KRAKx_GvZVapiUvoRYy:-KRAKx_HInlYALPhDZ1C:bmiepmf) ([raw file](https://github.com/uproxy/uproxy/blob/f742aafd2064704a9232bc665e53c29035c87e8f/src/generic_ui/hotpot_demo.html#L40)):*\n> ```HTML\n>     const HomeView = document.registerElement('home-view', HomeViewUnwrapped);\n> \n>     function makeTranslator(language) {\n> ```\n\n@lucyhe - thoughts on this design?  Do you think we should be passing in a translator function to each Polymer element, or should we continue using the ```{{ \"SOME_LABEL\" | $$(model.globalSettings.language) }}``` syntax, which relies on the global $$ function?  If we pass in a translator function as is done here, what happens if the user switches languages after they are past the welcome screen?\n\n---\n\n*[src/generic_ui/hotpot_demo.html, line 72 \\[r1\\]](https://reviewable.io:443/reviews/uproxy/uproxy/2755#-KRALsdBQSIWmfZsthGE:-KRALsdCV9uCnGDWa0CW:bp4o1y1) ([raw file](https://github.com/uproxy/uproxy/blob/f742aafd2064704a9232bc665e53c29035c87e8f/src/generic_ui/hotpot_demo.html#L72)):*\n> ```HTML\n>     }\n> \n>     function main() {\n> ```\n\nThis new design looks like it's going to be moving the element creation from the HTML to JavaScript.   For example, where you now create a WelcomeView in JavaScript, I could imagine with the older design we would have added this to the HTML:\n```<uproxy-welcome languageList='{{languageList}}' selectedLanguage='{{selectedLanguage}}' onLanguageSelected='{{onLanguageSelected}}' ...etc...>```\n\nDo you imagine we will want to move much more element creation to JS?  Some examples:\n* if the HomeView includes a settings drawer, do you think that would be created in the HTML for HomeView, or some corresponding JS?\n* for repeated elements (e.g. each user in the roster), do you expect these to move from ```<template repeat...``` tags to JS ```for``` loops?\n\n---\n\n*[src/generic_ui/hotpot_demo.html, line 72 \\[r1\\]](https://reviewable.io:443/reviews/uproxy/uproxy/2755#-KRAQGwKz9lyphHSWgrZ:-KRAQGwKz9lyphHSWgr_:b-qu1yn5) ([raw file](https://github.com/uproxy/uproxy/blob/f742aafd2064704a9232bc665e53c29035c87e8f/src/generic_ui/hotpot_demo.html#L72)):*\n> ```HTML\n>     }\n> \n>     function main() {\n> ```\n\nRelated to the above, is the reasoning behind creating elements in JS just that we want to avoid creating elements we don't need and using the ```hidden``` attribute?  If we use this type of design, I think eventually we will need to add the logic where the user has already seen the welcome screen and should advance to some other view - in that case do you imagine checking ```globalSettings.hasSeenWelcome``` in JavaScript and not creating the WelcomeView if it's true?  Or would you create the WelcomeView anyways, add it to the stack but then immediately add something else on top of it?\n\n---\n\n*[src/generic_ui/hotpot_demo.html, line 75 \\[r1\\]](https://reviewable.io:443/reviews/uproxy/uproxy/2755#-KRALMsNWyTVGR8sC3NC:-KRALMsNWyTVGR8sC3ND:bwybwd5) ([raw file](https://github.com/uproxy/uproxy/blob/f742aafd2064704a9232bc665e53c29035c87e8f/src/generic_ui/hotpot_demo.html#L75)):*\n> ```HTML\n>       let settings = new Settings();\n>       let stack_panel = new StackPanel(document.getElementById('root-panel'));\n>       let welcome_view = new WelcomeView();\n> ```\n\nIt seems like there are a lot of attributes on WelcomeView that are required in order for it to work properly (languageList, selectedLanguage, etc).  Can those be added to the constructor?  Can they be listed as attributes on the polymer-element (similar to elements we currently have, like https://github.com/uProxy/uproxy/blob/master/src/generic_ui/polymer/faq-link.html#L13)\n\n---\n\n*[src/generic_ui/views/metrics-choice-view.html, line 1 \\[r1\\]](https://reviewable.io:443/reviews/uproxy/uproxy/2755#-KRAKmVplzt--CdAjbBP:-KRAKmVplzt--CdAjbBQ:bmwzxyd) ([raw file](https://github.com/uproxy/uproxy/blob/f742aafd2064704a9232bc665e53c29035c87e8f/src/generic_ui/views/metrics-choice-view.html#L1)):*\n> ```HTML\n> <link rel=\"import\" href=\"/third_party/bower/paper-button/paper-button.html\">\n> ```\n\n@jpevarnek - can you look over this pull request and verify if anything here won't work with Polymer 1.0?\n\n---\n\n*[src/generic_ui/views/metrics-choice-view.html, line 19 \\[r1\\]](https://reviewable.io:443/reviews/uproxy/uproxy/2755#-KRARNyjn3ghMxhk4vMi:-KRARNyjn3ghMxhk4vMj:b6ljfe1) ([raw file](https://github.com/uproxy/uproxy/blob/f742aafd2064704a9232bc665e53c29035c87e8f/src/generic_ui/views/metrics-choice-view.html#L19)):*\n> ```HTML\n>         this.addEventListener('back', handler);\n>       },\n>       onDone: function(handler) {\n> ```\n\nI'm noticing some repetition between the files for this like onDone and the corresponding fireDone_ methods (in both metrics-choice-view and welcome-view).  If we continue using this pattern it might be nice to use some sort of helper functions to reduce the boiler-plate code.  Maybe something like this:\n```\nuProxyPolymer({...},  // 1st arg is the same as the object we currently pass to Polymer, except without onDone, fireDone_, onBack, fireBack_, onMetricsChoiceMade, fireMetricsChoiceMade_, etc\n['back', 'done', 'metrics-choice-made'] // 2nd arg is a list of signals\n)\n```\n```uProxyPolymer``` could then dynamically create an object to pass to ```Polymer(...)``` with the corresponding onBack, fireBack_, etc methods created.\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/uproxy/uproxy/2755#-:-KRAK02AfXUrSKBjgSb5:bap1e67)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"dborkan"}}