-
Notifications
You must be signed in to change notification settings - Fork 22
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
Preserve newlines from inbound and outbound sms. #168
base: main
Are you sure you want to change the base?
Conversation
package.json
Outdated
"turbo": "^1.2.16" | ||
}, | ||
"dependencies": { | ||
"react-textarea-autosize": "^8.4.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the tradeoffs of introducing a new dependency to the project?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, check out the npm workspaces documentation to learn more about how we're doing dependency management in this project. This is another one that can be a little tricky, but I think that this component might not be in the right place (it should probably be scoped to the dev phone ui's package.json
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should've added a comment with a question about adding this new dependency to the project. One tradeoff is adding more weight to our node_modeules and potentially reducing CI/CD execution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I agree. For the amount of functionality we need (a single input field) the package size might be a bit high - I think it's something like 1.2MB? Space isn't a huge issue for a dev tool, but we're just not using a lot of its functionality. Additionally, although this does seem fairly well maintained, it also can cause some strife if we upgrade to a breaking version of React, for example, and the project maintainers choose not to support it.
We currently use Paste for most of the Dev Phone, but I believe the Dialpad is an example custom component. Do you think it's possible to utilize an existing Paste component, or just write our own logic for this? I bet you can see how things are implemented in the react-textarea-autosize
component you found and use that as a basis for our own narrower implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we should probably create our own logic or use a past component. Down the road the external component could break, so it makes sense to build our own that we maintain.
I'm being a bit annoying unfortunately and asking about trade offs rather than writing a dissertation about what I think they are. 😂 So before we waste too much of your time on changes, I think we can step back and come up with the clear parameters for implementation. Namely, is it worth using an external component? |
Potentially better solution here: https://paste.twilio.design/components/chat-composer |
Replacing input tag with textarea tag to preserve newlines from inbound and outbound sms.
Closes #167
Contributing to Twilio