Skip to content

#345 Spacing variables for the css usage#367

Merged
marcinsawicki merged 3 commits intov1from
feature/345
Aug 1, 2022
Merged

#345 Spacing variables for the css usage#367
marcinsawicki merged 3 commits intov1from
feature/345

Conversation

@marcinsawicki
Copy link
Contributor

@marcinsawicki marcinsawicki commented Jul 27, 2022

Resolves: #345

Description

Scss variables of spacing, to the use in styling for components.

Storybook

Checklist

Obligatory:

  • Self-review
  • Unit & integration tests
  • Storybook cases
  • Design review
  • Functional (QA) review

Optional:

  • Accessibility cases (keyboard control, correct HTML markup, etc.)

@sgraczyk
Copy link

Why aren't we using CSS variables for that, or at least both with SCSS?

@marcinsawicki marcinsawicki added the feature New feature or request label Jul 27, 2022
@marcinsawicki marcinsawicki linked an issue Jul 27, 2022 that may be closed by this pull request
@use './themes/legacy';
@use './themes/light';
@use './themes/dark';
@use './spacing/spacing';

Choose a reason for hiding this comment

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

how about?:

Suggested change
@use './spacing/spacing';
@use './foundations/spacing';

import './index.scss';

export { DesignTokens } from './themes/designTokens';
export { SpacingTokens } from './spacing/spacingTokens';

Choose a reason for hiding this comment

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

Enumerations (even realized as consts) should have a singular form.

Suggested change
export { SpacingTokens } from './spacing/spacingTokens';
export { SpacingToken } from './foundation/spacing-tokens';

@@ -1,6 +1,7 @@
import './index.scss';

export { DesignTokens } from './themes/designTokens';
Copy link

@sgraczyk sgraczyk Jul 28, 2022

Choose a reason for hiding this comment

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

We could fix this also

Suggested change
export { DesignTokens } from './themes/designTokens';
export { DesignToken } from './themes/design-token';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 But we need to remember to include this as (small) breaking change in release docs, as we use DesignTokens enum in another app.

@@ -0,0 +1,16 @@
export const SpacingTokens = {

Choose a reason for hiding this comment

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

Alternatively SPACING_TOKENS, hard to say what's our approach here, as it seems more of a enumeration than a set of data.

Suggested change
export const SpacingTokens = {
export const SpacingToken = {

@marcinsawicki marcinsawicki requested a review from sgraczyk July 28, 2022 12:15
Copy link

@kstvsko kstvsko left a comment

Choose a reason for hiding this comment

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

I think everything is right, my only concern is the Examples in the Foundations/Spacing. I think we should address this during our new approach to the documentation to be more descriptive and clear, I will note it down.

@marcinsawicki marcinsawicki merged commit faa6495 into v1 Aug 1, 2022
@marcinsawicki marcinsawicki deleted the feature/345 branch August 1, 2022 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or request

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[Design Tokens] Spacing

3 participants