Feat/fct 16897/new tab layout #360
Conversation
🔍 Visual review for your branch is published 🔍Here are the links to: |
| color: { | ||
| primary: "text-foreground", | ||
| secondary: "text-secondary-foreground", | ||
| tertiary: "text-primary-foreground", | ||
| }, |
There was a problem hiding this comment.
Do we need these properties? I mean they don't hurt, but we default to using whatever color the container has. I guess if you don't set the property, it will just not add that class and still have this behavior?
There was a problem hiding this comment.
I can get rid of these, but I have 2 icons in the same container with different colors, so that forces me to wrap them and get it a bit dirtier, I'll push it so you can see what I mean.
|
|
||
| const Overview = () => ( | ||
| <div className="grid grid-cols-[2fr_1fr] divide-x divide-y-0 divide-dashed divide-muted"> | ||
| <AutoGrid tileSize="md" className="col-span-2 pl-10 pr-8 pt-6"> |
There was a problem hiding this comment.
We just recently added a Dashboard component meant for this use case! It just uses AutoGrid but I think it's better for scalability.
Don't hesitate to comment, the whole point of this is to agree on solid "foundations" to create a new and better version of Factorial right? So let's work on that, as long as we have the capacity, the better we can build this, the happier we'll be in 2025 and so on. |
That's the spirit! You made me cry 😭 |
| const Competencies = () => <div>Competencies</div> | ||
| const Activity = () => <div>Activity</div> | ||
|
|
||
| export const Default: Story = { |
There was a problem hiding this comment.
What would you feel about calling this whole story EmployeeProfile to make it more explicit? Tabs looks too generic to me.
There was a problem hiding this comment.
I was aiming for generic, my reasoning was that the same layout could be reused in a different screen.
There was a problem hiding this comment.
I understand! The thing is that this would make sense once we promote this to a component. Then we can find a meaningful name. If it lives in the playground, I think it's probably better to put it a very explicit name just to convey that's an "example".
I mean obviously not a blocker!
🚪 Why?
We are working on a new Profile with a whole new layout. We want to start this Layout in Factorial One and potentially reuse it in different screens.
🔑 What?
This Layout has a breadcrumb, a header, a tab navigation system and a content. The content will change when a different tab is selected. The Layout should be able to know how to handle this behaviour without including any content within itself, so it should be smart enough to navigate, but without needing to know what's inside.
Responsiveness is not required yet. This is my first PR on F1 so please feel free to point out any practices that could be improved. We want this to be a solid base for building future new screens in Factorial.
🏡 Context
https://www.figma.com/design/9pLsRzdinid0ha0qRWta2D/Employee-Profile?node-id=378-17717&m=dev
https://factorialmakers.atlassian.net/browse/FCT-16897
📸 Visual proof