feat: Add team section on company show#1119
Conversation
Co-authored-by: RubensRafael <rubensrafael2@live.com>
|
Hi 👋 Here's a preview environment 🚀 https://front-twentyhq-twenty-1119.env.ergomake.link Environment Summary 📑
Here are your environment's logs. For questions or comments, join Discord. Click here to disable Ergomake. |
|
Thanks! I've just tested with preview env and I can see that if you have many employees it won't fit properly in the menu Screen.Recording.2023-08-09.at.11.45.40.mov |
charlesBochet
left a comment
charlesBochet
left a comment
There was a problem hiding this comment.
Nice work! Added cosmetic comments
|
|
||
| const StyledTitle = styled.div` | ||
| color: ${({ theme }) => theme.font.color.primary}; | ||
| font-family: Inter; |
There was a problem hiding this comment.
font-family should not be needed
font-size: use theme
| font-family: Inter; | ||
| font-size: 13px; | ||
|
|
||
| font-style: normal; |
|
|
||
| const StyledTitleContainer = styled.div` | ||
| align-items: center; | ||
| align-self: stretch; |
There was a problem hiding this comment.
is align-self: strech needed?
|
|
||
| const StyledListContainer = styled.div` | ||
| align-items: flex-start; | ||
| align-self: stretch; |
|
|
||
| const StyledContainer = styled.div` | ||
| align-items: flex-start; | ||
| align-self: stretch; |
|
|
||
| font-style: normal; | ||
| font-weight: 500; | ||
| line-height: 150%; |
|
|
||
| const StyledCard = styled.div` | ||
| align-items: center; | ||
| align-self: stretch; |
There was a problem hiding this comment.
We are actually using this one. Using width: 100% would result in something like below:
| const StyledTitle = styled.div` | ||
| color: ${({ theme }) => theme.font.color.primary}; | ||
| font-family: Inter; | ||
| font-size: 13px; |
There was a problem hiding this comment.
use theme or remove css properties (see CompanyTeam comments)
| const StyledJobTitle = styled.div` | ||
| border-radius: ${({ theme }) => theme.spacing(1)}; | ||
| color: ${({ theme }) => theme.font.color.secondary}; | ||
| padding: 3px 4px 3px 0px; |
There was a problem hiding this comment.
let's not use 3px for values actually. Let's use theme spacing too
| color: ${({ theme }) => theme.font.color.primary}; | ||
| display: flex; | ||
| justify-content: space-between; | ||
| padding: 12px 12px 0px 12px; |
There was a problem hiding this comment.
let's use theme spacing for padding
On it |
Co-authored-by: RubensRafael <rubensrafael2@live.com> Co-authored-by: v1b3m <vibenjamin6@gmail.com>
Co-authored-by: RubensRafael <rubensrafael2@live.com> Co-authored-by: v1b3m <vibenjamin6@gmail.com>
|
Fixed companies with many members: team-members.mp4 |
3 participants
Fixes #1106
Demo:
https://www.loom.com/share/683ed55894254dec97bc04b60b5f9377?sid=64e569fd-4e7e-431b-a7e5-dd2bcadc3666