Add a button to expand the composer to full screen when the input has 3+ lines#9031
Add a button to expand the composer to full screen when the input has 3+ lines#9031stitesExpensify merged 60 commits intomainfrom
Conversation
|
Currently, expanding and collapsing the composer only works after refreshing the page, so I'll focus on fixing that. Please let me know what you think of my solution so far. |
|
Expanding and collapsing the composer works now. Next I want to automatically collapse the composer when a message is sent. Then I need to fix a bug where the composer doesn't reduce size when deleting lines from 3 to 0. |
|
iOS and Web are working well right now. Next I'll add support for Android and then I'll check to make sure that the other uses of the Composer (editing, emojis) still function correctly. |
|
Re-rolling PullerBear since I'm eager to get this merged and Luke is OOO for today. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by @stitesExpensify in version: 1.1.78-0 🚀
|
|
🚀 Deployed to production by @sketchydroide in version: 1.1.78-8 🚀
|
| componentWillUnmount() { | ||
| clearTimeout(this.loadingTimerId); | ||
| if (window.visualViewport) { | ||
| window.visualViewport.removeEventListener('resize', this.viewportOffsetTop); |
There was a problem hiding this comment.
@neil-marcellini nice work on this PR and feature. I came across this use of window in a cross-platform component and wanted to call out this section of the README.
We really shouldn't be seeing window unless we are in an index.js file and there's an index.native.js that is doing a no-op.
There was a problem hiding this comment.
Ok, got it. Is it worth making an index.native.js version of this component to fix this? Or could I make a withViewportOffsetTop HOC that would have a web and native version?
There was a problem hiding this comment.
Both could work. Though, this component is pretty important and having two might get confusing. I would create a small lib that returns something like VisualViewport we can call addEventListener() on and for mobile it would just return an empty function.
There was a problem hiding this comment.
Could also return a cleanup method something like
index.js
function addResizeListener(callback) {
if (!window.visualViewport) {
return () => {};
}
window.visualViewport.addEventListener('resize', callback);
return () => window.visualViewport.removeEventListener('resize', callback);
}index.native.js
function addResizeListener(callback) {
return () => {};
}I feel like I've used this pattern before elsewhere to get around the cross-platform guidance.
|
👋 Heads up! As per the BugZero checklist, commenting to surface that this introduced a regression on Android outlined here. |
|
Thanks for the heads up. From the videos in the PR I'm pretty sure everything was aligned when this was merged, but I guess my changes were fragile. |
Details
When a user has typed 3+ lines into the composer (chat text input), show an expand button that will expand the composer to be full screen. The user can also collapse the composer if they like. The composer will automatically collapse when the message is sent. If the composer is full screen and the user deletes lines until there are less than 3, the composer will remain expanded until they press the collapse button.
Fixed Issues
$ #8423
Tests / QA Steps
All platforms:
Sign into NewDot with any account
Native Mobile only:
PR Review Checklist
Contributor (PR Author) Checklist
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */displayNamepropertythisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)PR Reviewer Checklist
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick).src/languages/*filesSTYLE.md) were followed/** comment above it */displayNamepropertythisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)Screenshots
Web
web.mp4
Mobile Web
Android / Chrome
Android-Chrome.mp4
**iOS / Safari **
iOS-Safari.mp4
Desktop
desktop.mp4
iOS
ios.mp4
Android
android.mp4