[Md Feedback] Add initial data population for dialog.

chrome/browser/resources/md_feedback/feedback.js

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+cr.define('feedback', function() {

[afakhry, 2016/08/11 23:31:31]

First, thank you for the detailed comments and the clean code in this file. I'm
not against this chrome-style way of defining scoping and objects, however, the
more you work with me, you'll find that I'm a big fan of simplicity. I feel this
way is a bit hairy (but I understand it is common in our chromium code base).
But what if we can do better? :)

Compare the code here to the simple one in this gPaste:
https://paste.googleplex.com/5630126029012992

Which one do you like looking at and maintaining more?
I'm not a JS expert at all, that's why I prefer simpler code. So if there's any
advantage to this old chromium-style to the one in the gPaste, please let me
know.

[
Note that you'd have to rename your |kSetData| in the cc file to:
constexpr char kSetData[] = "Feedback.UI.setData";
]

[apacible, 2016/08/12 01:46:41]

The snippet is definitely much simpler!

The style here is the recommended format in the docs:
https://www.chromium.org/developers/webui

I'm strongly inclined to follow the general style for consistency with the rest
of the chrome/browser/resources/* files (and also possibly because I'm more used
to reading this type of format), but am otherwise unsure of a 'technical
advantage' here.

[afakhry, 2016/08/12 19:22:38]

Bear in mind that the webui doc you pointed out is probably too old and hasn't
been updated in years! :D
So this style was probably needed when we couldn't use the newer, cleaner ES6
class syntax. Now that we can, we should probably consider using it.
By the way, I was the first to land a JS class syntax in our codebase. Please
see:
https://cs.chromium.org/chromium/src/chrome/browser/resources/feedback/js/eve....

As I mentioned earlier, I'm not a JS expert. So in order to decide what we
should do here, when you come back, we can consult with dbeam@ about his
opinion.

Enjoy your vacation! :)

[apacible, 2016/08/29 17:00:20]

+dbeam for thoughts (and/or future of WebUI guidelines as well)

[Dan Beam, 2016/09/14 05:11:21]

what do you want my specific thoughts on?  'use strict';?  ES6 classes?

I don't think the linter currently understands ES6 features, and support is
dodgy on iOS (or at least differs from everywhere else because it's basically
mobile Safari instead of Chrome).

I have a bug to remove the linter somewhere, as it's kinda old and unmaintained.
 there has been efforts to replace it with clang-format and jscompiler.jar
--js_comp_warn=lintChecks, but it's not quite the same.

in general: if your code only runs on desktop and you don't care about
presubmits preventing you from CQ'ing your changes, feel free to use ES6
features.

+  'use strict';
+
+  /**
+   * The feedback-container element. Initialized after Polymer is ready.
+   * @type {?FeedbackContainer}
+   */
+  var container = null;
+
+  /**
+   * Initialize the Feedback WebUI by requesting data to populate the current
+   * feedback form.
+   */
+  function initialize() {
+    container = /** @type {!FeedbackContainerElement} */
+        ($('container'));
+
+    feedback.browserApi.requestData();
+  }
+
+  return {
+    initialize: initialize,
+  };
+});
+
+// API invoked by the browser MdFeedbackWebUIMessageHandler to communicate
+// with this UI.
+cr.define('feedback.ui', function() {
+  'use strict';
+
+  /**
+   * Populates the feedback form with data.
+   *
+   * @param {{email: string|undefined,
+   *          url: string|undefined}} data
+   * Parameters in data:
+   *   email - user's email, if available.
+   *   url - url of the tab the user was on before triggering feedback.
+   */
+  function setData(data) {
+    container.email = data['email'];
+    container.url = data['url'];
+  }
+
+  return {
+    setData: setData,
+  };
+});
+
+// API invoked by this UI to communicate with the browser WebUI message
+// handler.
+cr.define('feedback.browserApi', function() {
+  'use strict';
+
+  /**
+   * Requests data to initialize the WebUI with.
+   * The data will be returned via feedback.ui.setData.
+   */
+  function requestData() {
+    chrome.send('requestData');
+  }
+
+  return {
+    requestData: requestData,
+  };
+});
+
+window.addEventListener('load', feedback.initialize);

[afakhry, 2016/08/11 23:31:31]

Do you need to wait for the page to fully load (DOM, images, stylesheets,
...etc.) to initialize the form data? If not, then please use 'DOMContentLoaded'
instead of 'load'.

(I also want to note the difference visually, if possible. With load, the email
and url are filled after the page shows up).

[apacible, 2016/08/12 01:46:41]

Done.