Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove margin from ContentNotification #352

Merged
merged 1 commit into from
Dec 19, 2018

Conversation

dferber90
Copy link
Contributor

@dferber90 dferber90 commented Dec 19, 2018

Our ui-kit components shouldn't define their own margins. The spacing should always be defined by the parent. We therefore remove the margin defined on ContentNotification.

Consumers can use Spacings.Inset to wrap ContentNotification instead. Use scale="m" to keep the same spacing as originally defined on ContentNotification.

<Spacings.Inset scale="m">
  <ContentNotification>Hello there</ContentNotification>
</Spacings.Inset>

@dferber90 dferber90 added the 💅 Type: Enhancement Improves existing code label Dec 19, 2018
Copy link
Member

@emmenko emmenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!! 🙏

Copy link
Contributor

@tdeekens tdeekens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UI-Kit elements themselves should not have spacings applied the outside.

@dferber90 dferber90 force-pushed the df-improve-content-notification branch from a3d2e9f to a488da5 Compare December 19, 2018 16:08
@dferber90 dferber90 merged commit 06675d1 into master Dec 19, 2018
@dferber90 dferber90 deleted the df-improve-content-notification branch December 19, 2018 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💅 Type: Enhancement Improves existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants