Updates node dependencies via npm audit

conorsch commented 5 years ago

Begins the process of updating the node dependencies based on output from npm audit. There's now a CI check in the "safety" workflow that will fail until npm audit reports no problems.

See the commit messages for details on what's been done so far. Still a few dangling updates to be made, so requesting collab from @harrislapiroff. Don't hesitate to drop commits in here if you'd prefer to reset and update the deps in a more nuanced fashion.

harrislapiroff commented 5 years ago

As far as I'm concerned this branch is ready! However, since it now contains as much of my code as anyone else's, I'd like to get @chigby's eyes on it—particularly my last few commits converting it from gulp to webpack. Some testing guidelines:

[ ] Ensure everything runs without errors locally, using docker-compose
[ ] Ensure the website javascript and CSS renders
[ ] Ensure fonts look right and icons look right
[ ] Ensure you can make changes to javascript and CSS and see those reflected on the site

conorsch commented 5 years ago

Took a pass through in the local dev env. Results:

[x] Ensure everything runs without errors locally, using docker-compose
[x] Ensure the website javascript and CSS renders
[ ] :red_circle: (see below) Ensure fonts look right and icons look right
[ ] :large_orange_diamond: (did not test) Ensure you can make changes to javascript and CSS and see those reflected on the site

Noticed one regression on the styles, namely that the grades on the leaderboard are no longer colored according to score. See screenshots below.

On this branch, with webpack

stn-grades-uncolored

On master branch

stn-grades-colored-2

That was the only problem that was obvious to me. As noted above, I didn't test in-place modifications of the styles—but I'll consider a fix on the style regression here proof that we can indeed change them. :smiley:

chigby commented 5 years ago

Looks like the problem @conorsch is seeing is due to changes with string interpolation introduced by pug 2.0, notably that it now uses standard JS string templates:

We removed support for interpolation in attributes since the implementation was unnecessarily complex, and the feature tended to discourage users from learning that they can just use any JavaScript value in place of attributes.

See the migration guide for more info.

Basically, we need to make this change:

--- a/client/src/leaderboardtemplate.jade
+++ b/client/src/leaderboardtemplate.jade
@@ -34,7 +34,7 @@ mixin check(condition)
               a(href=item.absolute_url)= item.name
               if item.pledge
                 a(href=item.pledge.url)
-                  i.pledged.fa.fa-handshake-o(title="#{ item.name } have pledged to secure their site.")
+                  i.pledged.fa.fa-handshake-o(title=`${item.name} have pledged to secure their site.`)
             td.desktop-only
               +check(item.valid_https)
             td.desktop-only
@@ -46,6 +46,6 @@ mixin check(condition)
             td.desktop-only
               +check(item.hsts_preloaded)
             td
-              span(class='grade #{item.grade.class_name}')= item.grade.grade
+              span(class=`grade ${item.grade.class_name}`)= item.grade.grade

conorsch commented 5 years ago

Thanks, @chigby! One more request. While I can confirm that the change you pushed up resolved the color regression on the full leaderboard display, it still occurs on the homepage (which my initial report did not mention):

Live site in prod

stn-homepage-leaderboard-color

On this branch

stn-homepage-leaderboard-nocolor

It's not immediately obvious to me where the corresponding change should be made—can you take a look?

conorsch commented 5 years ago

Gorgeous. @chigby's latest changes resolve all styling issues, based on local testing. Looks good!

freedomofpress / securethenews