Feat: add my message component #8
Conversation
src/MyMessage/index.tsx
Outdated
| const topRightRadius = | ||
| !lastMessage || lastMessage.sender_username !== message.sender_username | ||
| ? '1.3em' | ||
| : '0.3em'; | ||
| const bottomRightRadius = | ||
| !nextMessage || nextMessage.sender_username !== message.sender_username | ||
| ? '1.3em' | ||
| : '0.3em'; |
There was a problem hiding this comment.
some repeated logic- maybe create a helper function
There was a problem hiding this comment.
The logic is a bit different, I typically enforce a refactor 3 or more times.
~1.5 times is typically fine with me
There was a problem hiding this comment.
ya I also find giving something a name makes the code easier to read but will leave up to you
src/MyMessage/index.tsx
Outdated
|
|
||
| setConfiguration({ maxScreenClass: 'xl' }); | ||
|
|
||
| export const MyMessage: React.FC<Props> = ({ |
There was a problem hiding this comment.
thinking ahead to TheirMessage - I'm thinking that there will be some logic that can be shared between the two e.g. renderFiles, renderImages, renderReads
idk how it's done in typescript but in Java we would use an Abstract Class for a base Message class then have MyMessage and TheirMessage extend it
at the very least they can be helper functions that both classes have access to
There was a problem hiding this comment.
Yeah and good catch, the new ImageThumb and FileThumb are refactored in now.
I'm going to pick this apart a bit more tomorrow, I think it can be a bit better.
|
Oh FYI I'm not done. There is quite a bit I'm gonna clean before doing the review. I just have a habit of pushing and making the PR early lol |
size-limit report 📦
|
ahhh all good- just lmk when it's ready for review |
|
I think it's ready for v1 review now |
| first_name: string | null; | ||
| last_name: string | null; | ||
| avatar: string | null; | ||
| custom_json: string | object | null; |
There was a problem hiding this comment.
Custom JSON is a Chat Engine feature https://www.youtube.com/watch?v=EcC6fA2Umn8&t=2s
| created: string; | ||
| } | ||
| export interface MessageProps { | ||
| id?: number; |
There was a problem hiding this comment.
Good point. I got some time after work to do this 👍
There was a problem hiding this comment.
Yeah so when the last_message attribute in a chat objects may not have an ID if the message is blank.
This is something we can patch with the API later but now it's optional
| const renderImages = () => { | ||
| const attachments = | ||
| message && message.attachments ? message.attachments : []; | ||
|
|
||
| return attachments.map((attachment, index) => { | ||
| const fileName = getFileName(attachment.file); | ||
|
|
||
| if (isImage(fileName)) { | ||
| return ( | ||
| <ImageThumb | ||
| attachment={attachment} | ||
| isLoading={isSending || attachment.file === null} | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| return <div key={`attachment${index}`} />; | ||
| }); | ||
| }; | ||
|
|
||
| const renderFiles = () => { | ||
| const attachments = | ||
| message && message.attachments ? message.attachments : []; | ||
|
|
||
| return attachments.map((attachment, index) => { | ||
| const fileName = getFileName(attachment.file); | ||
|
|
||
| if (!isImage(fileName)) { | ||
| return ( | ||
| <FileThumb | ||
| attachment={attachment} | ||
| isLoading={isSending || attachment.file === null} | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| return <div key={`attachment${index}`} />; | ||
| }); | ||
| }; |
There was a problem hiding this comment.
I feel like this could be combined to a renderAttachments function instead of looping over the list of attachments twice you could just do if/else on the isImage and do it all in one go
There was a problem hiding this comment.
Good point. I got some time after work to do this 👍
There was a problem hiding this comment.
Actually wait there's a good reason for this too: Basically we want all the images rendered, then all the Files rendered. If we do it all in one loop then the files/images render in a mixed order which we don't want UI wise
No description provided.