Conversation
🦋 Changeset detectedLatest commit: c5f8f55 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
There was a problem hiding this comment.
Pull request overview
Adjusts the Timeline component’s visual spacing to better align TimelineBody content with surrounding Timeline elements, addressing primer issue #6438.
Changes:
- Increases
TimelineBodytop margin by 1px viacalc()to improve vertical alignment. - Adds a targeted stylelint disable for the spacing rule on the updated declaration.
| max-width: 100%; | ||
| margin-top: var(--base-size-4); | ||
| /* stylelint-disable-next-line primer/spacing */ | ||
| margin-top: calc(var(--base-size-4) + 1px); |
There was a problem hiding this comment.
Adding the 1px calc feels icky to me, try with.
| margin-top: calc(var(--base-size-4) + 1px); | |
| margin-top: 0.25lh; |
There was a problem hiding this comment.
We generally avoid using lh (and don't currently have any uses in our repo) because of Safari compatibility issues. We'd have to suppress the lint warning and provide a fallback value to cover unsupported browsers, which ends up being less clean overall. This calc() pattern is what we usually use when we need an in-between value not directly provided by our existing CSS variables, so I lean toward keeping things consistent.
|
@copilot add patch changeset |
|
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/14686 |
|
integration tests passing but VRT status is not reporting here. Added skip label to unblock this PR |
Closes https://github.com/github/primer/issues/6438
Changelog
Changed
Added +1px to the
margin-topofTimelineBodyto improve visual alignment of elementsRollout strategy
Testing & Reviewing
Good test stories:
Merge checklist