Music: timeline fix

[breville]

This makes a small adjustment to avoid the vertical overflow of timeline elements.

before

after

[breville]

LGTM! Is it worth a brief code comment explaining the magic number 2?

@hannahbergam I'm glad you asked, because I didn't have a great answer beyond it working for this fairly obvious case.

We work out the event height here, and part of that height is a gap at the bottom, calculated here. It turns out that with recent layout changes, our bar height is 132px but we actually get 127px to render these events.

I wrote a little code to output each row count, and its respective remaining vertical space, with negative space implying an overflow. I then adjusted the bar height down until we didn't get any overflow.

for (var rowCount = 5; rowCount <= 45; rowCount++) { 
  let eventHeight = Math.floor(128 / rowCount); 
  let gap = eventHeight > 8 ? 3 : eventHeight > 6 ? 2 : 1; 
  console.log(rowCount + ':', 127 - (eventHeight * rowCount - gap))  
}

It turns out that compensating by 4 pixels (i.e. assuming the bar height is 128px) was the magic number, so I've updated that.

[hannahbergam]

LGTM! Is it worth a brief code comment explaining the magic number 2?

@hannahbergam I'm glad you asked, because I didn't have a great answer beyond it working for this fairly obvious case.

We work out the event height here, and part of that height is a gap at the bottom, calculated here. It turns out that with recent layout changes, our bar height is 132px but we actually get 127px to render these events.

I wrote a little code to output each row count, and its respective remaining vertical space, with negative space implying an overflow. I then adjusted the bar height down until we didn't get any overflow.

for (var rowCount = 5; rowCount <= 45; rowCount++) { 

  let eventHeight = Math.floor(128 / rowCount); 

  let gap = eventHeight > 8 ? 3 : eventHeight > 6 ? 2 : 1; 

  console.log(rowCount + ':', 127 - (eventHeight * rowCount - gap))  

}

It turns out that compensating by 4 pixels (i.e. assuming the bar height is 128px) was the magic number, so I've updated that.

Oh wild- thank you for the explanation and the update! Looks good.