fix: align timeline component dimensions based on size prop#49
fix: align timeline component dimensions based on size prop#49hitomihiumi wants to merge 2 commits into
Conversation
|
@hitomihiumi is attempting to deploy a commit to the Dopler Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
This PR refactors the Timeline component’s sizing/spacing logic (introducing shared size/offset tokens and some horizontal padding tweaks) and updates the Text component documentation to remove the deprecated/unsupported type prop from the API reference.
Changes:
- Refactor
Timelinemarker sizing and offset calculations into shareddimensionSize/topOffsetvalues. - Adjust
Timelinemarker container spacing (e.g., marginTop and horizontal edge padding) and reformat conditional props for readability. - Remove
typefrom the Text docs API reference table.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/core/src/components/Timeline.tsx | Refactors marker sizing/offset handling and adjusts layout props for vertical/horizontal timeline rendering. |
| apps/docs/src/content/once-ui/components/text.mdx | Updates Text component API reference to remove type prop. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const dimensionSize = | ||
| size === "xs" ? "8" : size === "s" ? "24" : size === "m" ? "32" : size === "l" ? "40" : "48"; | ||
| const topOffset = | ||
| size === "xs" ? "20" : size === "s" ? "12" : size === "m" ? "8" : size === "l" ? "4" : "0"; |
| <PropsTable | ||
| content={[ | ||
| ["as", "ElementType", `"span"`], | ||
| ["variant", "TextVariant"], | ||
| ["size", ["xs", "s", "m", "l", "xl"]], | ||
| ["weight", ["default", "strong"]], | ||
| ["type", ["display", "heading", "body", "label", "code"]], | ||
| ["family", ["display", "heading", "body", "label", "code"]], | ||
| ["onBackground", "`${ColorScheme}-${ColorWeight}`"], | ||
| ["onSolid", "`${ColorScheme}-${ColorWeight}`"], |
|
Nice fix! I took a different approach — instead of recalculating the offset values, I restructured the vertical layout so the dot and label share a row with vertical="center", removing the need for manual offset math entirely. Both solve the alignment, yours is more minimal though. @lorant-one just fyi 
|
|
I think your fix will be better |
The indentation height is now dynamic and depends on the size of the marker. #43
Without fix:
With fix:
