-
Notifications
You must be signed in to change notification settings - Fork 0
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
fix: add padding-block and padding-inline properties #399
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for pine-design-system ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
* Defines the vertical alignment of the box items. | ||
* @defaultValue start | ||
*/ | ||
@Prop() alignSelf?: `start` | `center` | `end` | `baseline` | `stretch`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const element = page.root; | ||
expect(element).toHaveClass(`pds-padding-inline-${size}`); | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are no specs for alignSelf
as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requesting changes because the prop alignSelf
is not being used within the component.
bbe39d3
to
7d2cb47
Compare
@@ -283,7 +283,79 @@ $pine-spacing-tokens: ( | |||
padding: var(--pine-dimension-2xl); | |||
} | |||
|
|||
.pds-padding-block-none { | |||
padding-block: 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick but could this use --pine-dimension-none
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some nitpicks, but overall LGTM! 👍🏼
} | ||
|
||
.pds-padding-inline-none { | ||
padding-inline: 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same --pine-dimension-none
here?
Description
This issue is to remedy a request that came into DS-Support
padding-block
andpadding-inline
to allow for more padding flexibilityalign-self
for alignment flexibility within flex containersFixes DSS-1309
Type of change
How Has This Been Tested?
Please describe the tests you've added and run to verify your changes.
Provide instructions so that we can reproduce.
Please also list any relevant details for your test configuration.
Test Configuration:
Checklist:
If not applicable, leave options unchecked.