-
Notifications
You must be signed in to change notification settings - Fork 0
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
Frontend PR #3
base: main2
Are you sure you want to change the base?
Frontend PR #3
Conversation
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.
- why are some component files
.jsx
while other.js
? - move the component folder out of the pages folder.
- too many extra line spaces and indentation issues.
context/UsersContext.jsx
Outdated
const UsersContext = createContext(); | ||
|
||
export function UsersWrapper({ children }) { | ||
const [user,setUser]= useState([]) |
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.
why is user an array?
pages/components/ChatBox.jsx
Outdated
padding: 1rem !important; | ||
|
||
|
||
` |
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.
spacing
pages/components/ChatBox.jsx
Outdated
const [mObj,setmObj]=useState({}) | ||
const [messageList,setMessageList]= useState([]) | ||
const messagesEndRef = useRef(null) | ||
const context= useUsersContext() |
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.
avoid genric naming
pages/components/ChatBox.jsx
Outdated
const [socketState, setSocketState]=useState() | ||
//console.log("We've Re-rendered") | ||
|
||
let socketRef= null |
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.
will sockedState
and socketRef
be always the same?
pages/components/ChatBox.jsx
Outdated
message:event.target.value, | ||
to:selectedContact, | ||
from:context.user.name, | ||
time:new Date().toLocaleTimeString([], |
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.
why not set these params before sending messages instead of setting them again and again on each character change?
pages/components/ChatBox.jsx
Outdated
</div> | ||
</ContactColumn> | ||
|
||
<ChatColumn style={{border:'1px solid black', 'border-radius':'10px'}}> |
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.
avoid inline styles
pages/components/ChatBox.jsx
Outdated
{ | ||
messageList?.map( message => { | ||
if(message.from===selectedContact || (message.from === context.user.name && message.to=== selectedContact)) | ||
return <div key={dummy()} style={{overflow: 'hidden'}}> |
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.
provide valid key
pages/components/ChatBox.jsx
Outdated
if(user.name !== context.user.name){ | ||
return ( | ||
<ContactListItem | ||
key={dummy()} |
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.
provide valid key
pages/components/ChatBox.jsx
Outdated
if(message.from===selectedContact || (message.from === context.user.name && message.to=== selectedContact)) | ||
return <div key={dummy()} style={{overflow: 'hidden'}}> | ||
{ message.from=== context.user.name ? | ||
<Message style={{float:'right',backgroundColor:'#E1EBEE'}}>{ message.message }<span>{message.time}</span></Message> |
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.
no inline styles
pages/components/ChatBox.jsx
Outdated
</div> | ||
}) | ||
} | ||
<div ref={messagesEndRef} /> |
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.
can you find a better way to get auto-scroll functionality .
No description provided.