Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clamping of consecutive elements with a common parent is broken #2

Closed
codener opened this issue Sep 6, 2019 · 3 comments
Closed

Clamping of consecutive elements with a common parent is broken #2

codener opened this issue Sep 6, 2019 · 3 comments
Assignees
Labels

Comments

@codener
Copy link
Member

codener commented Sep 6, 2019

Consider this markup:

<div class="common-parent">
  <div class="first" clamp>Some ... text</div>
  <div class="second" clamp>Some ... text</div>
</div>

In this situation, Superclamp will not always clamp the .second element properly, i.e. its contents will exceed its height and bleed out of .second. Unfortunately, this behavior is not deterministic, meaning it will sometimes clamp right and sometims do it wrong.

When investigating a broken .second, I found that Superclamp had stored incorrect elementAt values for the element.

I assume this originates from the fact that Superclamp splits "measurement" and "adaption" in two consecutive phases. This breaks when the measures for .second are changed during the adaption of .first.

@codener codener added the bug label Sep 6, 2019
@makmic
Copy link
Member

makmic commented Oct 24, 2019

I managed to create a minimal example, where the second of two subsequent elements is not clamped correctly.

c646b6c
image

As this error only occurs when using max-height, my naive guess would be to process clamping jobs ordered via LIFO, that the second element is always processed before the first. (Code)

drainPhaseQueue = (phase) ->
  jobs = jobQueues[phase]
  if jobs.length > 0
-    while job = jobs.shift()
+    while job = jobs.pop()
      job()

Sadly, this does not fix the issue. Any other ideas?

@makmic
Copy link
Member

makmic commented Oct 24, 2019

One more hint: Disabling value-caching fixes the problem, but is not an option:

- instance = element[INSTANCE_KEY] || new Superclamp(element)
+ instance = new Superclamp(element)

makmic added a commit that referenced this issue Feb 26, 2020
@makmic makmic self-assigned this Feb 26, 2020
foobear added a commit that referenced this issue Mar 20, 2020
…-height` caused subsequent elements to be clamped incorrectly.

Resolves #2
foobear added a commit that referenced this issue Mar 20, 2020
…-height` caused subsequent elements to be clamped incorrectly.

Resolves #2
foobear added a commit that referenced this issue Mar 20, 2020
…-height` caused subsequent elements to be clamped incorrectly.

Resolves #2
@foobear
Copy link
Member

foobear commented Mar 23, 2020

Fixed in 0.2.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants