search

bigcommerce / catalyst

Catalyst - for Composable Commerce
https://catalyst-demo.site
MIT License
117 stars 153 forks source link

fix: move margins to button in slideshow #1643

Closed andrewreifman closed 2 weeks ago

andrewreifman commented 2 weeks ago

What/Why?

In the Slideshow component, if a slide description is removed, there was no spacing between the title and button. Margin classes were changed to ensure the title, description, and button had the proper spacing between them at all times.

Testing

View the Slideshow component and hide any slide description.

changeset-bot[bot] commented 2 weeks ago

⚠️ No Changeset found

Latest commit: 992cd8ef9b0dea18b3ea87fa09528542f4578395

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

vercel[bot] commented 2 weeks ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
catalyst-latest ❌ Failed (Inspect) 💬 Add feedback Nov 15, 2024 6:07pm
5 Skipped Deployments | Name | Status | Preview | Comments | Updated (UTC) | | :--- | :----- | :------ | :------- | :------ | | **catalyst** | ⬜️ Ignored ([Inspect](https://vercel.com/bigcommerce-platform/catalyst/BtNB44a3qTEPGHxszXMJaoj3X9sY)) | | | Nov 15, 2024 6:07pm | | **catalyst-au** | ⬜️ Ignored ([Inspect](https://vercel.com/bigcommerce-platform/catalyst-au/ASdDxMnH4b3mKjrH6cmp8iX6ENUf)) | [Visit Preview](https://catalyst-au-git-andrew-style-polish-bigcommerce-platform.vercel.app) | | Nov 15, 2024 6:07pm | | **catalyst-soul** | ⬜️ Ignored ([Inspect](https://vercel.com/bigcommerce-platform/catalyst-soul/BXicUNjg7wajPRmDan5tWKWdoAoX)) | [Visit Preview](https://catalyst-soul-git-andrew-style-polish-bigcommerce-platform.vercel.app) | | Nov 15, 2024 6:07pm | | **catalyst-uk** | ⬜️ Ignored ([Inspect](https://vercel.com/bigcommerce-platform/catalyst-uk/3wGoTbUTdBhfwcvECWZatjgEATuY)) | [Visit Preview](https://catalyst-uk-git-andrew-style-polish-bigcommerce-platform.vercel.app) | | Nov 15, 2024 6:07pm | | **catalyst-unstable** | ⬜️ Ignored ([Inspect](https://vercel.com/bigcommerce-platform/catalyst-unstable/7PcptFmBYYw4V88AwcagYBqnsqXi)) | [Visit Preview](https://catalyst-unstable-git-andrew-style-polish-bigcommerce-platform.vercel.app) | | Nov 15, 2024 6:07pm |
andrewreifman commented 2 weeks ago

pnpm-lock shouldn't need to be updated. Component changes look good tho!

@jorgemoya I assume it changed the lock file by me just running pnpm install. Do I just need to undo that change in the file and push it back up?

I also was about to ask in Slack what I should do about the lint failures in the build that aren't related to my PR.

jorgemoya commented 2 weeks ago

pnpm-lock shouldn't need to be updated. Component changes look good tho!

@jorgemoya I assume it changed the lock file by me just running pnpm install. Do I just need to undo that change in the file and push it back up?

I also was about to ask in Slack what I should do about the lint failures in the build that aren't related to my PR.

I would just undo the file and push it back up. We can then investigate if we need to update the lock file.

andrewreifman commented 2 weeks ago

@jorgemoya This is ready for another review

jorgemoya commented 2 weeks ago

@andrewreifman I still see the lock file in the PR :(