Adding support for onLayoutChange notifier configuration

[sean-roberts]

We are trying to allow an integrator to know about the layout state of our sidebar as it changes.
The signature we are expecting is:

window.hypothesisConfig = ()=>{
  return {
    onLayoutChange: ( { width, height, expanded } )=>{}
  };
};

As noted above we will provide the width, height, and the expanded state.

[codecov]

Codecov Report

Merging #499 into master will increase coverage by <.01%.
The diff coverage is 90.9%.

@@            Coverage Diff             @@
##           master     #499      +/-   ##
==========================================
+ Coverage   90.74%   90.74%   +<.01%     
==========================================
  Files         135      135              
  Lines        5361     5383      +22     
  Branches      931      938       +7     
==========================================
+ Hits         4865     4885      +20     
- Misses        496      498       +2

Impacted Files	Coverage Δ
src/annotator/config/index.js	100% <ø> (ø)	⬆️
src/annotator/sidebar.coffee	85.84% <90.9%> (+1.22%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2cb95be...0f2f8b0. Read the comment docs.

[robertknight]

Rebased on master.

In testing this, I see that when opening or closing the sidebar the width reported is the one before the state changed. This means that if an integrator tries to adjust the position of some elements when the sidebar becomes wider after it is opened or closed, it will end up in the wrong place:

Example output logged by an onLayoutChanged callback when opening and then closing the sidebar:

Sidebar state changed 37x316. Open: true
Sidebar state changed 465x316. Open: false

[robertknight]

Testing using localhost:3000, the callback is not invoked when the client initially loads if openSidebar is set to false.

Steps to reproduce:

1. Modify live-reload-server.js's hypothesisConfig setup code to:

window.hypothesisConfig = function () {
  return {
    liveReloadServer: 'ws://' + appHost + ':${port}',
    onLayoutChange({ width, height, expanded }) {
      console.log(\`Sidebar state changed \${width}x\${height}. Open: \${expanded}\`);
    }
  };
};

2. Restart gulp and browse to http://localhost:3000

Expected: Callback invoked when client initially loads with { width: <collapsed width>, height: <sidebar height>, expanded: false }.
Actual: No log messages printed.

[robertknight]

I found two issues in testing and left some minor comments on the code:

1. The callback does not appear to be invoked when the client initially loads if the sidebar is not configured to open by default.
2. When opening and closing the sidebar, the width reported in the callback is the width before the sidebar state changed rather than the width after the sidebar finishes opening/closing. In order for integrators to position elements correctly, we need to either tell them what the final state will be or trigger another callback once the open/close animation completes.

[robertknight]

My CoffeeScript syntax highlighter complains if lines end with semicolons. The compiler doesn't complain but we should omit them for consistency.

[robertknight]

Would it be possible to avoid hardcoding this number?

[robertknight]

I wonder if we should rename this to onSidebarLayoutChange to be more explicit about what thing's layout changed?

[sean-roberts]

Sidebar is probably redundant. it's not sidebarBranding, etc. Let's keep it the way it is 👍

[sean-roberts]

Updated the width algorithm to calculate properly when we toggle expanded states. I updated the expanded state to also reflect if we are above the fully collapsed width - allowing panning to properly update expanded or not.

Also, made the padding for the left side of the sidebar dynamic based on the width of the toolbar

[robertknight]

LGTM. I added a doc comment to _notifyOfLayoutChange and an internal comment about the structure of the sidebar and how expanding/collapsing it is achieved by adjusting the left margin of the container, since this existing aspect of how the sidebar works wasn't immediately obvious to me.

We'll need to make sure we document this, along with any other features added to support Readium's requirements.