krasimir / simpre

HTML presentation framework
MIT License
38 stars 2 forks source link

Scaling algorithm: correct operator in `if` condition? #4

Closed rauschma closed 6 months ago

rauschma commented 6 months ago

File view.js. Is the || in line (*) intended?

  let top = false;
  let left = false;
  if (wAfterScale < vw - 10) {
    top = 0;
    left = (vw - wAfterScale) / 2;
  }
  if (hAfterScale < vh - 10) {
    top = (vh - hAfterScale) / 2;
  }
  if (top !== false || left !== false) { // (*)
    node.style.position = 'absolute';
    node.style.top = top + 'px';
    node.style.left = left + 'px';
  }

Shouldn’t it be &&? With ||, .topor .left may be set to 'falsepx'. This may also be an option:

  let top = undefined;
  let left = undefined;
  if (wAfterScale < vw - 10) {
    top = 0;
    left = (vw - wAfterScale) / 2;
  }
  if (hAfterScale < vh - 10) {
    top = (vh - hAfterScale) / 2;
  }
  node.style.position = 'absolute';
  node.style.top = (top ?? 0) + 'px';
  node.style.left = (left ?? 0) + 'px';
rauschma commented 6 months ago

The following seems to work too, but I may be overlooking something:

const wAfterScale = node.getBoundingClientRect().width;
const hAfterScale = node.getBoundingClientRect().height;
node.style.position = 'absolute';
node.style.top = Math.max(0, (vh - hAfterScale) / 2) + 'px';
node.style.left = Math.max(0, (vw - wAfterScale) / 2) + 'px';
krasimir commented 6 months ago

@rauschma you are absolutely right. I replaced the nonsense with the simpler:

const wAfterScale = node.getBoundingClientRect().width;
const hAfterScale = node.getBoundingClientRect().height;
node.style.position = 'absolute';
node.style.top = Math.max(0, (vh - hAfterScale) / 2) + 'px';
node.style.left = Math.max(0, (vw - wAfterScale) / 2) + 'px';

I'm not really sure what I meant by writing that old version. Now, when I'm looking at it seems confusing.

Just released a new version with the change.

P.S. Do you use the SimPre for presentations. It's in my arsenal for quite some time and I'm wondering if/when someone is finding it useful.

rauschma commented 6 months ago

Now, when I'm looking at it seems confusing.

A frequent experience of mine with my code. 😀

Do you use the SimPre for presentations?

I’m working on a server for live-reloading Markdown slides that also supports audience polls (embedded in slides). I used SimPre’s slide scaling algorithm to get started and it’s been very helpful.