Video 4454 message list scrolling #467
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
timmydoza
reviewed
Mar 24, 2021
Comment on lines
52
to
56
| * This component is a container for the MessageList component. It enables scrolling so that users | ||
| * can scroll through the messages as the MessageList grows taller than this container. | ||
| * | ||
| * Note that this component is tested with Cyprus only. | ||
| */ |
There was a problem hiding this comment.
I think the PR is good to go, but I think these comments could be a little more descriptive. I think the PR description is a good example of what could go here.
There was a problem hiding this comment.
thank you! i also spelled "Cypress" wrong 🙃
timmydoza
approved these changes
Mar 24, 2021
olipyskoty
commented
Mar 24, 2021
Comment on lines
+52
to
+61
| * This component is a scrollable container that wraps around the 'MessageList' component. | ||
| * The MessageList will ultimately grow taller than its container as it continues to receive | ||
| * new messages, and users will need to have the ability to scroll up and down the chat window. | ||
| * A "new message" button will be displayed when the user receives a new message, and is not scrolled | ||
| * to the bottom. This button will be hidden if the user clicks on it, or manually scrolls | ||
| * to the bottom. Otherwise, this component will auto-scroll to the bottom when a new message is | ||
| * received, and the user is already scrolled to the bottom. | ||
| * | ||
| * Note that this component is tested with Cypress only. | ||
| */ |
There was a problem hiding this comment.
@timmydoza do you mind proofreading this? want to make sure it's not too big
There was a problem hiding this comment.
Looks good! Not too big. Can't have too many comments 🙂
timmydoza
pushed a commit
that referenced
this pull request
Mar 31, 2021
* VIDEO 3731 Conversations UI - create chat window (#421) * Video 4078 add flip camera button to mobile menu (#433) VIDEO 4078 Conversations - Add flip camera button to 'more' mobile menu * Video 3730 add conversation hooks (#438) * Add initial hooks for chat * Update hooks in chat provider * Add tests for conversationProvider component * Fix DeviceSelectionScreen tests * Update variable names * Revert proxy url in package.json * Fix linting issues * Make error handling more user friendly * Fix a test in src/state * Temporarily disable cypress tests * Undo prettier changes to config.yml * Update chatProvider test * Cleanup ChatProvider * Video 3732 Conversations UI - create message components (#441) * Add links to TextMessage component (#446) * Add links to TextMessage component * Update CSS in TextMessage * Update snapshot test * Fix issue in addLinks function * Use forEach loop in addLinks function * Add new server (#443) * Use new server with reused code * Add firebase auth to server * Convert server to typescript * Add tests for firebaseAuthMiddleware * Refactor server * Add more server tests * Add tests for CreateExpressHandler * Fix linter issue * Delete old server * Fix linter issues * Fix some server issues * Set up server build steps * Change request to match new server api * Fix token fetch test * Add server build step to circleci config * Enable cyress tests in CI * Undo prettier changes in config.yml * install plugin-rtc from npm * Fix firebaseAuth test * Add create_conversation:true to token requests * Upgrade plugin-rtc to 0.8.0 * Temorarily remove firebaseAuthMiddleware and recording rules route * Add ChatProvider back in * Update .env.example * Change build:server script * Add firebaseAuthMiddleware * Add comments to firebaseAuthMiddleware * Update useFirebaseAuth tests * Video 3728 Conversations UI - add input field and send button (#451) * Chat notification (#454) * Add notification to chat icon * Add tests for ToggleChatButton * Some cleanup * Fix linting issue * Video-4525 loading screen (#461) * Video 4454 Conversations UI - message list scrolling (#467) * Update MessageList component to change how messages are grouped (#469) * Update MessageList component to change how messages are grouped * Simplify MessageList code * Video 3888 file input for conversations (#458) * Add FileAttachmentIcon * Add File Input to ChatInput component * Add change handler to file input * Create MediaMessage component and add it to MessageList * Toggle ChatWindow visibility with CSS * Update messageList tests * Add function to format the file size * Update error handling in ChatInput component * Add tests for MediaMessage component * Update snapshot test * Add tests for ChatInput * Fix linting issue * Update filesize formatting * Update error language and tests * Reset file input value after file sent * Center icon in MediaMessage * Update Icon size * Fix breaking ChatInput test * Update snapshot * Use accept attribute on file input element * Update readme to add information about the chat feature and conversations api * Cross browser fixes (#472) * Update app.yaml for the new server * Add cache controls for static assets * Only hide Screenshare button on mobile devices * Auto focus on chat input when user opens chat window * Fix iOS bug with media messages * Fix issues in MessageListScrollContainer * Fix app.yaml * Fix infinite loop when there is a conversations error * Update tests for ChatInput * Update mediamessage test * Update menubar tests * Update messageList snapshot * Fix linting issue * Add comments to the server * Video 3727 - cypress testing (#473) * Video 4597 feature flag (#475) * Update plugin-rtc version * Add feature flag for Twilio Conversations * Add tests * Add info to .env.example * Update readme with info about twilio conversations (#476) * Update readme with info about twilio conversations * Update readme to address feedback * Release 0.3.0 (#479) * Bump version and update changelog * Add plugin-rtc upgrade instructions to the readme Co-authored-by: olipyskoty <77076398+olipyskoty@users.noreply.github.com> Co-authored-by: Sean Coleman <smcoleman27@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Contributing to Twilio
Pull Request Details
JIRA link(s):
Description
This PR is for a new component that is a scrollable container to be wrapped around the
<MessageList />component. The<MessageList />will ultimately grow taller than its container as it continues to receive new messages, and users will need to have the ability to scroll up and down the chat window. A "new message" button will be displayed if the user receives a new message and is not scrolled to the bottom. The button will disappear if the user clicks on the button, or manually scrolls to the bottom. Otherwise, the chat window will auto-scroll to the bottom if a new message is received, and the user is already scrolled to the bottom.I've included some comments in the code, but let me know if any of it is overkill, or if I should add more.
Screenshot:
Burndown
Before review
npm testBefore merge