-
Notifications
You must be signed in to change notification settings - Fork 237
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
Adjusted the sidebar width to cover the entire width for small screens #630
Changes from 1 commit
1cd83a8
8899246
666e778
3b8fdc9
67201f9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
import React, { useState } from 'react'; | ||
import React, { useState, useEffect } from 'react'; | ||
import { isSameDay, format } from 'date-fns'; | ||
import { Box, Sidebar, Popup, useTheme } from '@embeddedchat/ui-elements'; | ||
import { MessageDivider } from '../../Message/MessageDivider'; | ||
|
@@ -41,14 +41,39 @@ export const MessageAggregator = ({ | |
|
||
const noMessages = messageList?.length === 0 || !messageRendered; | ||
const ViewComponent = viewType === 'Popup' ? Popup : Sidebar; | ||
const [isSmallScreen, setIsSmallScreen] = useState(window.innerWidth <= 780); | ||
|
||
useEffect(() => { | ||
const handleResize = () => { | ||
setIsSmallScreen(window.innerWidth <= 780); | ||
}; | ||
|
||
window.addEventListener('resize', handleResize); | ||
return () => window.removeEventListener('resize', handleResize); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest using media queries instead. In my opinion, a resize event listener is not needed at all. Consider how mobile devices work—will they be resizing their screens? |
||
}, []); | ||
|
||
const viewComponentStyle = isSmallScreen | ||
? { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please don't define styles here. There is nothing wrong with it, but we follow a specific structure where all major styles are in the component.style.js file, and only inline styles should be applied in the component.js file. Look at other components and please follow the same pattern. |
||
backgroundColor: theme.colors.background, | ||
position: 'absolute', | ||
width: '100%', | ||
left: 0, | ||
zIndex: 1, | ||
} | ||
: { | ||
backgroundColor: theme.colors.background, | ||
width: '100%', | ||
left: 0, | ||
zIndex: 1, | ||
}; | ||
|
||
return ( | ||
<ViewComponent | ||
title={title} | ||
iconName={iconName} | ||
searchProps={searchProps} | ||
onClose={() => setExclusiveState(null)} | ||
style={{ padding: 0 }} | ||
style={viewComponentStyle} | ||
{...(viewType === 'Popup' | ||
? { | ||
isPopupHeader: true, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,12 @@ | ||
import React, { useContext } from 'react'; | ||
import React, { useContext, useEffect, useState } from 'react'; | ||
import { css } from '@emotion/react'; | ||
import { | ||
Box, | ||
Avatar, | ||
Sidebar, | ||
Popup, | ||
useComponentOverrides, | ||
useTheme, | ||
} from '@embeddedchat/ui-elements'; | ||
import RCContext from '../../context/RCInstance'; | ||
import { useChannelStore } from '../../store'; | ||
|
@@ -23,13 +24,39 @@ const Roominfo = () => { | |
return `${host}/avatar/${channelname}`; | ||
}; | ||
|
||
const { theme } = useTheme(); | ||
const ViewComponent = viewType === 'Popup' ? Popup : Sidebar; | ||
const [isSmallScreen, setIsSmallScreen] = useState(window.innerWidth <= 780); | ||
|
||
useEffect(() => { | ||
const handleResize = () => { | ||
setIsSmallScreen(window.innerWidth <= 780); | ||
}; | ||
|
||
window.addEventListener('resize', handleResize); | ||
return () => window.removeEventListener('resize', handleResize); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment as above ! |
||
}, []); | ||
|
||
const viewComponentStyle = isSmallScreen | ||
? { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment as above ! |
||
backgroundColor: theme.colors.background, | ||
position: 'absolute', | ||
width: '100%', | ||
left: 0, | ||
zIndex: 1, | ||
} | ||
: { | ||
backgroundColor: theme.colors.background, | ||
width: '100%', | ||
left: 0, | ||
zIndex: 1, | ||
}; | ||
return ( | ||
<ViewComponent | ||
title="Room Information" | ||
iconName="info" | ||
onClose={() => setExclusiveState(null)} | ||
style={viewComponentStyle} | ||
{...(viewType === 'Popup' | ||
? { | ||
isPopupHeader: true, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,7 +23,6 @@ const RoomMembers = ({ members }) => { | |
const { host } = ECOptions; | ||
const { theme } = useTheme(); | ||
const styles = getRoomMemberStyles(theme); | ||
|
||
const toggleInviteView = useInviteStore((state) => state.toggleInviteView); | ||
const showInvite = useInviteStore((state) => state.showInvite); | ||
const [isLoading, setIsLoading] = useState(true); | ||
|
@@ -50,11 +49,39 @@ const RoomMembers = ({ members }) => { | |
const roles = userInfo && userInfo.roles ? userInfo.roles : []; | ||
const isAdmin = roles.includes('admin'); | ||
const ViewComponent = viewType === 'Popup' ? Popup : Sidebar; | ||
|
||
const [isSmallScreen, setIsSmallScreen] = useState(window.innerWidth <= 780); | ||
|
||
useEffect(() => { | ||
const handleResize = () => { | ||
setIsSmallScreen(window.innerWidth <= 780); | ||
}; | ||
|
||
window.addEventListener('resize', handleResize); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment as above ! |
||
return () => window.removeEventListener('resize', handleResize); | ||
}, []); | ||
|
||
const viewComponentStyle = isSmallScreen | ||
? { | ||
backgroundColor: theme.colors.background, | ||
position: 'absolute', | ||
width: '100%', | ||
left: 0, | ||
zIndex: 1, | ||
} | ||
: { | ||
backgroundColor: theme.colors.background, | ||
width: '100%', | ||
left: 0, | ||
zIndex: 1, | ||
}; | ||
|
||
return ( | ||
<ViewComponent | ||
title="Members" | ||
iconName="members" | ||
onClose={() => setExclusiveState(null)} | ||
style={viewComponentStyle} | ||
{...(viewType === 'Popup' | ||
? { | ||
isPopupHeader: true, | ||
|
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.
Is there any specific reason for changing this? The current structure uses a specific set of z-index values for different elements, so it behaves correctly during integration. Is it necessary to change it? Please look for ways to avoid this change
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.
The z-Index is made 1 so that the date for the messages does not appear over sidebar menu when it gets adjusted to Full Screen