.keepwithnext and figures fallback interaction #290

Closed
andreacampi opened this Issue Jul 12, 2012 · 2 comments

Projects

None yet

1 participant

@andreacampi
Member

tl;dr I think that Page.fillColumn should skip fallback for figure that have already been placed when looking for the next block.

Consider an article like this:

<article>
  <h1 class="keepwithnext">Section Title</h1>

  <figure>
    <div data-sizes="double"><img src="image1"></div>
    <div data-sizes="fallback">
      <p>Short text</p>
    </div>
  </figure>

  <figure>
    <div data-sizes="double">….</div>
    <div data-sizes="fallback">
      <p>Text 1</p>
      <p>Text 2</p>
  </div>
  </figure>
</article>

And a very simple grid with a container for the double plus two columns.

As long as the height is sufficient to fit everything, this works nicely; this example is too simple to show it off, but .keepwithnext really works nicely with figure fallback content, too.

As you make the browser windows shorter, this breaks down and you end up with the "Section Title" in one column and "Text 1/Text 2" in the second one.
This happens because of an inconsistency in Page.fillColumn:

  • block 0 (Section Title) is not processed yet, its nextSibling is the next figure; when we reach Page#L509 we'll check for available space using its firstLine, and it may very well fit. If that happens we continue the loop, adding the text to the column and continueing the loop;
  • block 1(

    Short text

    ) is a figure fallback and it's already processed, so the check at line 442 will pass and we'll skip over to the next block;
  • block 2 is much taller than we expect and it won't fit! and by now it's too late to undo block 0.
@andreacampi andreacampi added a commit that referenced this issue Jul 12, 2012
@andreacampi andreacampi [#290] When looking for the next block for keepwithnext we need to ex…
…clude fallback content for figures that have already been placed.
f1dc5ec
@andreacampi
Member

See https://github.com/andreacampi/treesaver/tree/issue-290 for my attempt at fixing this.
Existing tests pass; writing a specific test for this sounds like a daunting task though.

I'm not sure this is a complete fix. Page.nextNotUsedBlock probably needs to use getNextNonChildBlock instead of simply iterating on blocks?

@andreacampi andreacampi added a commit to andreacampi/treesaver that referenced this issue Jul 16, 2012
@andreacampi andreacampi [#290] When looking for the next block for keepwithnext we need to ex…
…clude fallback content for figures that have already been placed.
3ed9c87
@andreacampi andreacampi added a commit to andreacampi/treesaver that referenced this issue Aug 25, 2012
@andreacampi andreacampi [#290] When looking for the next block for keepwithnext we need to ex…
…clude fallback content for figures that have already been placed.
4780398
@andreacampi
Member

Fix merged.

@andreacampi andreacampi added a commit to andreacampi/treesaver that referenced this issue Oct 16, 2012
@andreacampi andreacampi [#290] When looking for the top margin of the next sibling we need to…
… skip fallbacks for figures that have already been used.
acbcb8a
@andreacampi andreacampi added a commit that referenced this issue Oct 16, 2012
@andreacampi andreacampi [#290] When looking for the top margin of the next sibling we need to…
… skip fallbacks for figures that have already been used.
73c799f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment