PRD-742/fix:Settings tab should have 2 scrollable sections #9218
Conversation
| @@ -108,7 +112,7 @@ export const SettingsPage = () => { | |||
| <Menu | |||
| selectable={false} | |||
| mode="inline" | |||
| style={{ width: 256, marginTop: 8 }} | |||
| style={{ width: 256, marginTop: 8, overflowY: 'auto', overflowX: 'hidden' }} | |||
There was a problem hiding this comment.
can we extract into a const object above?
There was a problem hiding this comment.
yes we can do it. Changes done are pushed
| @@ -19,6 +20,13 @@ interface Props extends TabsProps { | |||
| onTabChange?: (selectedTab: string) => void; | |||
| } | |||
|
|
|||
| const RoutedTabsStyle = styled.div` | |||
| max-height: 100%; | |||
There was a problem hiding this comment.
this has impacts across the application. it is widely used. i would recomend we pls try to avoid changing this if possible. why is this needd?
There was a problem hiding this comment.
we can remove max-height it will work same but can't remove flex property because it creating container for inside item and when there is more items it help us to add scroll inside this container. Changes are pushed
|
If we set flex property at a parent container only left and right will only scroll. But What I'm doing is just make section scroll-able which has more data. So if we have more data in tab section only scroll added to that section not whole right section same with table if there is more data in table only table will be scroll-able not the whole right section |
| @@ -8,6 +8,11 @@ import { UserList } from './user/UserList'; | |||
| const PageContainer = styled.div` | |||
| padding-top: 20px; | |||
| width: 100%; | |||
| max-height: 100%; | |||
There was a problem hiding this comment.
why is this max height necessary?
There was a problem hiding this comment.
it doesn't seem necessary in other places
There was a problem hiding this comment.
I have kept max-height because if there is lots of data then container can take only 100% height but yes flex doing the same thing so we can remove height.
| @@ -20,7 +23,11 @@ const PageTitle = styled(Typography.Title)` | |||
| } | |||
| `; | |||
|
|
|||
| const ListContainer = styled.div``; | |||
| const ListContainer = styled.div` | |||
| display: flex; | |||
There was a problem hiding this comment.
Is this one necesary to have display: flex?
or flex-direction?
overflow i think makes sense
There was a problem hiding this comment.
should we use flex: 1 to fill up the remaining space?
There was a problem hiding this comment.
If we remove Flex from ListContainer scroll will go this upper container i have also added video to slack let me know if you have suggestion
| @@ -23,11 +28,15 @@ const PageTitle = styled(Typography.Title)` | |||
| `; | |||
|
|
|||
| const Content = styled.div` | |||
| height: 100%; | |||
There was a problem hiding this comment.
can we use flex: 1 instead?
There was a problem hiding this comment.
I have kept height because if there is lots of data then container can take only 100% height but yes flex doing the same thing so we can remove height.
|
I have remove unnecessary height from code but we required flex property to get the scroll on required sections |
Checklist