#hypothes.is

hypothesis/h#9208 (2728-refactor-annotation-controller-tests - 2c1953f : Sean Hammond): The build passed.

Change view : https://github.com/hypothesis/h/compare/23a07a4...

Build details : https://travis-ci.org/hypothesis/h/builds/94687871

<seanh> nick: robertknight I need this to get on with writing tests for my new code https://github.com/hypothesis/h/pull/2763 if either of you has time to take a quick look

<seanh> It's just sanitizing the `AnnotationController` tests a little (believe me there's still plenty of wrong in there)

<robertknight> seanh: Taking a look now

<robertknight> GitHub thinks the diff is huge to the point of not showing it

<seanh> Yeah I indented the whole file by two characters

<robertknight> Any idea of GitHub has a `git diff -w` mode?

<robertknight> seanh: Merged

<seanh> Cheers!

<judell> "@judell: See above ^. Are you happy that I should create a card for this and put in the current sprint?" Of course. :face_with_head_bandage:

<robertknight> So I'm looking at how best to make the design consistent across the app and WordPress and make it easy to keep the two in sync. The approach I'm looking at which I think can work quite nicely is to re-use npm. There will be an npm package containing the base styles (defining fonts, colors, typography), icon fonts and shared components (eg. the nav bar) which consists of scripts, HTML templates and a small amount of JS. This will live in the m

repository and be required in package.json in the wordpress-hypothesis-theme repo. The Gulpfile which builds the assets for the WordPress theme and the WordPress templates will pull in the relevant base styles and the nav bar. A consequence of this is that updating the nav bar will not be possible from WordPress' graphical interface - but that's not a problem since we want it to be consistent with the app anyway.

<seanh> I'm a little surprised that this `0 && !0 && ('' || [])` evaluates to `0` in JavaScript

<seanh> Isn't it a boolean expression?

<robertknight> Which simplifies to `0 && true => 0`

<robertknight> That is interesting, I wouldn't have got that pub quiz answer right

<seanh> Actually what we have in the code is `0 && !0 && !('' || [])`

<robertknight> Yes, but the above is a simpler version that shows the same effect

<seanh> Which seems like it could be 0 && !0 && !'' && ![]`

<seanh> Yeah, I wouldn't expect `0 && true` to be `0`

<robertknight> Ah, so the semantics are that `x && y` returns `x` if x is falsey

<robertknight> https://developer.mozilla.org/en-US/docs/Web/Ja...

<robertknight> This is why testing for exactly equality to falsey values is hazardous in JS

<seanh> Yeah

<seanh> It's also why you should write unit tests so that you actually know what your methods return

<seanh> Ok, this is ready now too https://github.com/hypothesis/h/pull/2762

hypothesis/h#9217 (2728-refactor-highlight-saving - 4d69bec : Sean Hammond): The build has errored.

Change view : https://github.com/hypothesis/h/compare/dca1d14...

Build details : https://travis-ci.org/hypothesis/h/builds/94704393