0.8 preview

[atotic]

Lots of new code here. Removed google-analytics-base, because no inheritance. google-analytics-setup is used instead. google-dashboard is gone, replaced by x-autobind inside demo.html. It had fundamental event ordering isssues in 0.8, replacement was easier. Everything should work.

[philipwalton]

@atotic I tried to checkout this PR and run it locally, and I got a lot of errors, even with a completely fresh bower install. Is there a trick to get all these dependencies in the right 0.8-preview state?

Also, the <google-analytics-dashboard> element can't go away because other projects that consume these elements depend on it, and it's a big selling point of the library.

Let me know if you have any questions. I'm happy to work with you to help get these elements upgraded.

[philipwalton]

Can you update the demo to use <google-analytics-dashboard> so I can easily test that all the pieces are working together?

[atotic]

done.

On Mon, Apr 27, 2015 at 5:40 PM, Philip Walton notifications@github.com wrote:

Can you update the demo to use so I can easily test that all the pieces are working together?

— Reply to this email directly or view it on GitHub https://github.com/GoogleWebComponents/google-analytics/pull/24#issuecomment-96859724 .

[atotic]

also, ::shadow and /deep/ just got depreciated, we'll see what the workarounds are....

[philipwalton]

Yeah, I was planning on addressing the ::shadow styles, but I think it makes sense to do that in a separate PR since <x-style> is still working at the moment.

[philipwalton]

I can't make a line note since it didn't change, but maxResults in demo.html should be max-results. See the line here: https://github.com/atotic/google-analytics/blob/0.8-preview/demo.html#L206

[atotic]

Nice catch on maxResults. These get me all the time. Fixed.

[philipwalton]

Okay, I've looked over all the source files and the logic all seems to work and is correct. There are a few stylistic things but I figured I could wait to make individual comments until all my current comments are addressed.

Specifically, there is inconsistent spacing between methods (style guide says 1 newline), and many of the public methods do not have JSDoc comments. @ebidel can give more feedback on what is required for this PR.

Let me know once you're done with all the other changes, and I can give it another once-over.

Thanks for all your help!

FYI, here's the style guide: https://docs.google.com/document/d/1LCWYPgUx78yrg8Got3WbRJMnowumXk38V-QQ0h1FZhM/edit#

[atotic]

Other changes are all done. I've also looked at method spacing, fixed everything that popped out.

Regarding documentation changes, I've agreed with Eric to give all docs a workover once iron-doc-viewer is stable. Lots of changes coming up (automatic property type and declaration inference), docs on all methods, private ones must have _, or be declared private, etc.

[atotic]

All done, except for ripping out the error message.

[ebidel]

Just some nits

[ebidel]

@philipwalton I'm happy with this if you are. LGTM from me.

[philipwalton]

I think the only remaining thing is to add 'use strict' to all elements and make sure the indentation of the IIFE is consistent. Otherwise LGTM.

[atotic]

Added 'use strict'. Not sure what IIFE is.

[philipwalton]

Sorry, IIFE is an immediately invoked function expression. In other words, all elements should be structured and indented like this:

<script>
  (function() {
    'use strict';
    Polymer({
      // ...
    });
  })();
</script>

[atotic]

Then I think I am in the clear.

On Mon, May 4, 2015 at 3:51 PM, Philip Walton notifications@github.com wrote:

Sorry, IIFE is an immediately invoked function expression. In other words, all elements should be structured and indented like this:

— Reply to this email directly or view it on GitHub https://github.com/GoogleWebComponents/google-analytics/pull/24#issuecomment-98879119 .

[atotic]

Currently, demo chart display is failing in 0.5, and 0.8 code. I suspect something on the server. Suspending work, will try again when it works.

[atotic]

service restored. Fixes pushed.

[philipwalton]

LGTM.