Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

MM-14579 Correctly size AutosizeTextarea on mount #2531

Merged
merged 2 commits into from
Mar 28, 2019
Merged

Conversation

jwilander
Copy link
Member

@jwilander jwilander commented Mar 22, 2019

Summary

Depending on the order of events when the post/comment textboxes were being mounted, sometimes the height of the textbox would not be set due to races with when the post-create div was mounted. I removed the use of jquery to add a class name and instead used state in the CreatePost and CreateComment components to add the class as needed and made sure to call the size calculation on textbox mount.

Ticket Link

https://mattermost.atlassian.net/browse/MM-14579

Checklist

  • Ran make check-style to check for style errors (required for all pull requests)
  • Ran make test to ensure unit and component tests passed
  • Added or updated unit tests (required for all new features)

@jwilander jwilander added the 2: Dev Review Requires review by a core commiter label Mar 22, 2019
Copy link
Member

@hmhealey hmhealey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Down with jquery magic!

Since onHeightChange is triggered on the AutosizeTextarea's component update and that triggers CreatePost and CreateComment to update, I'm a tiny bit worried about those two getting in a loop where they update each other. I don't think that can actually happen though since it would require some text that's long enough for the scrollbar to appear, and then when the scrollbar appears, the text height is short enough for the scrollbar to disappear again, but that shouldn't be possible since the scrollbar squishes the text horizontally and makes it taller.

@hmhealey hmhealey added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Mar 28, 2019
@hmhealey hmhealey added this to the v5.12.0 milestone Mar 28, 2019
@jwilander jwilander modified the milestones: v5.12.0, v5.10.0 Mar 28, 2019
@jwilander jwilander merged commit 9a6a29f into master Mar 28, 2019
@jwilander jwilander added the CherryPick/Approved Meant for the quality or patch release tracked in the milestone label Mar 28, 2019
@jwilander jwilander deleted the mm-14579 branch March 28, 2019 18:46
@jwilander jwilander added CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone and removed CherryPick/Approved Meant for the quality or patch release tracked in the milestone labels Mar 28, 2019
@amyblais amyblais added Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation labels Mar 28, 2019
stevepartridge pushed a commit to stevepartridge/mattermost-webapp that referenced this pull request Mar 30, 2019
thekiiingbob pushed a commit to thekiiingbob/mattermost-webapp that referenced this pull request Apr 2, 2019
@lindy65 lindy65 added Tests/Not Needed Does not require new release tests and removed 4: Reviews Complete All reviewers have approved the pull request labels Apr 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Changelog/Done Required changelog entry has been written CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone Docs/Not Needed Does not require documentation Tests/Not Needed Does not require new release tests
Projects
None yet
5 participants