• 
      

    Add Student Sonar frontend code.

    Review Request #6897 — Created Feb. 3, 2015 and submitted

    Information

    student-sonar
    master
    719c2fb...

    Reviewers

    The frontend is architected using the new hotness of ES6 and web components.
    There are two custom elements currently in use: x-student card, which provides
    a small clickable avatar + name, and x-student-view, which is a full-page view
    showing the student's name/school/email/avatar, and contains the different
    analytics for each student.

    At the moment, we show their participation on Slack using a github-like heatmap
    calendar, and a list of their review requests on Review Board. Future
    improvements will show code reviews, status reports, and links to their notes.

    There's a lot still to be improved here, but this is a good start.

    
     

    Description From Last Updated

    Blank line between these.

    brenniebrennie

    Blank line between these.

    brenniebrennie

    Nit - double-quotes for href and rel

    mike_conleymike_conley

    As mentioned in Slack, we'll need the shim-shadowdom attribute set here to activate the polyfills for things like /deep/.

    mike_conleymike_conley

    While this is ocurring, would it be worth displaying a spinner or loading message instead of just a blank page?

    mike_conleymike_conley

    innerText is non-standard. We should use textContent here.

    mike_conleymike_conley

    So I think I've found a cross-browser solution that allows us to avoid adding the template <style> rules to style.css. …

    mike_conleymike_conley

    I assume, reading the rest of these rules, that an updated CSS spec says that we don't need a semi-colon …

    mike_conleymike_conley

    Nit - spaces after :

    mike_conleymike_conley
    reviewbot
    1. Tool: Pyflakes
      Ignored Files:
          index.html
          lib/frontend/x-student-view.js
          lib/frontend/main.js
          lib/frontend/x-student-card.js
          lib/frontend/components.js
          style.css
          README.md
      
      
      
      Tool: PEP8 Style Checker
      Ignored Files:
          index.html
          lib/frontend/x-student-view.js
          lib/frontend/main.js
          lib/frontend/x-student-card.js
          lib/frontend/components.js
          style.css
          README.md
      
      
    2. 
        
    brennie
    1. 
        
    2. lib/frontend/x-student-view.js (Diff revision 1)
       
       
       
      Show all issues

      Blank line between these.

    3. lib/frontend/x-student-view.js (Diff revision 1)
       
       
       
      Show all issues

      Blank line between these.

    4. 
        
    david
    reviewbot
    1. Tool: PEP8 Style Checker
      Ignored Files:
          index.html
          lib/frontend/x-student-view.js
          lib/frontend/main.js
          lib/frontend/x-student-card.js
          lib/frontend/components.js
          style.css
          README.md
      
      
      
      Tool: Pyflakes
      Ignored Files:
          index.html
          lib/frontend/x-student-view.js
          lib/frontend/main.js
          lib/frontend/x-student-card.js
          lib/frontend/components.js
          style.css
          README.md
      
      
    2. 
        
    mike_conley
    1. 
        
    2. index.html (Diff revision 2)
       
       
      Show all issues

      Nit - double-quotes for href and rel

    3. index.html (Diff revision 2)
       
       
      Show all issues

      As mentioned in Slack, we'll need the shim-shadowdom attribute set here to activate the polyfills for things like /deep/.

    4. lib/frontend/main.js (Diff revision 2)
       
       
       
       
      Show all issues

      While this is ocurring, would it be worth displaying a spinner or loading message instead of just a blank page?

      1. Maybe theoretically, but in practice it loads quite quickly. I'll add a TODO.

    5. lib/frontend/x-student-card.js (Diff revision 2)
       
       
      Show all issues

      innerText is non-standard. We should use textContent here.

    6. lib/frontend/x-student-card.js (Diff revision 2)
       
       
      Show all issues

      So I think I've found a cross-browser solution that allows us to avoid adding the template <style> rules to style.css.

      Add a cssAdded boolean to the list of module variables at the top, then in createdCallback, after creating the clone:

              if (Platform.ShadowCSS && !cssAdded) {
                  let style = clone.querySelector('style');
                  let cssText = Platform.ShadowCSS.shimCssText(
                      style.textContent, tagName);
                  Platform.ShadowCSS.addCssToDocument(cssText);
                  cssAdded = true;
              }
      

      You can probably do it before the clone by querying template.content too, but I didn't try that.

    7. style.css (Diff revision 2)
       
       
      Show all issues

      I assume, reading the rest of these rules, that an updated CSS spec says that we don't need a semi-colon after the last rule... right?

      If so, I guess we don't need the semi-colon after block.

      1. This was all adapted from the cal-heatmap CSS, so I didn't even notice this. I prefer semicolons after each line because it's easier to maintain that way.

    8. style.css (Diff revision 2)
       
       
       
      Show all issues

      Nit - spaces after :

    9. 
        
    david
    reviewbot
    1. Tool: PEP8 Style Checker
      Ignored Files:
          index.html
          lib/frontend/x-student-view.js
          lib/frontend/main.js
          lib/frontend/x-student-card.js
          lib/frontend/components.js
          style.css
          README.md
      
      
      
      Tool: Pyflakes
      Ignored Files:
          index.html
          lib/frontend/x-student-view.js
          lib/frontend/main.js
          lib/frontend/x-student-card.js
          lib/frontend/components.js
          style.css
          README.md
      
      
    2. 
        
    david
    mike_conley
    1. Let's get this stuff landed and start dogfooding.

      Great job on this.

    2. 
        
    david
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to master (c459dda)