autosize icon indicating copy to clipboard operation
autosize copied to clipboard

resizing event

Open emirotin opened this issue 9 years ago • 7 comments

This is simple change that adds an event before we attempt to resize the textarea. I need it to save some state of the ui (specifically the scroll position of a different element) and restore it after the actual resize has happened (because the growing textarea shrinks that other element).

We later discovered that the conditional "resized" event caused bugs in our code because the scroll was still happening sometimes (probably when height was set to auto).

I also made tiny code style fixes for consistency, and small DRY refactoring.

emirotin avatar Mar 13 '16 12:03 emirotin

I am trying to solve a similar / the same problem: need to conditionally scroll a different element whose height the autosizing might have affected, depending on e.g. the previous height (and/or scroll). an autosize:beforeResized event, or similar. It's not clear how this change works given the description, but I'm in the middle of rebasing the code for merging to master.

nmschulte avatar Aug 10 '16 19:08 nmschulte

It's not clear how this change works given the description

Can you elaborate what's unclear? The change is basically about firing the "going to resize" event before the library decided if the actual resize is needed.

emirotin avatar Aug 10 '16 21:08 emirotin

Before I'd perused the code, your description wasn't clear as to how a user would use this change. After rebasing, it is clear: the change uses a new event autosize:resizing ~~re-uses (and conflates, given the event name) the same event, autosize:resized~~.

As well, it seems the event may fire even if autosize doesn't resize the element. Is that correct? That'd be my only concern~~, along w/ the naming conflation/confusion~~.

Definitely usable as it is though, hence the rebase and re-PR. Thanks for the work!

nmschulte avatar Aug 10 '16 21:08 nmschulte

No, it does not reuse the same name. It adds the new name, autosize:resizing.

Yes, this event for the sake of simplicity is fired even if the final resize doesn't happen. It reads like "going to resize right now. maybe"

emirotin avatar Aug 10 '16 21:08 emirotin

No, it does not reuse the same name. It adds the new name, autosize:resizing.

Ah, thanks for the replies, I totally missed that; I was too concerned w/ the change to always issue a autosize:resized event. I doubt @jackmoore is going to like that aspect of this. I don't quite understand the bug (where you suggest height: auto might be the issue) behind this, though. How is it that "the scroll was still happening"?

nmschulte avatar Aug 10 '16 21:08 nmschulte

Hi @emirotin, thanks for the original pull request. I think the last release may fix the issue you were working around by adding this event and saving your UI state. It preserves the scroll position on ancestor elements, which is what it sounds like you are manually controlling for.

My apologies for not responding before, I hadn't personally experienced the problem you were having at that time, and so didn't understand the need for an additional event.

jackmoore avatar Aug 10 '16 21:08 jackmoore

I don't quite understand the bug (where you suggest height: auto might be the issue) behind this, though. How is it that "the scroll was still happening"?

I don't remember the details, TBH. But the fact is the conditional check was failing and the event wasn't fired, though the scroll did happen in the sibling element. Maybe it's because of the double resize or something, which caused the sibling to shrink and then instantly grow to the previous height. But it will detach the scroll:

  • element height is 100, scrollTop is 100
  • element height is 90, scrollTop is 90 (the browser tries to keep it at 100, but fixes the edge case)
  • element height is 100 again, scrollTop is still at 90

emirotin avatar Aug 11 '16 08:08 emirotin