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.

Sign up for GitHub

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

Jump to bottom

feat(tokens): add typography based sizing #2796

Open
wants to merge 35 commits into
base: next
Choose a base branch
from
Open

feat(tokens): add typography based sizing #2796

wants to merge 35 commits into from

Conversation

Febakke
Copy link
Member

@Febakke Febakke commented Nov 19, 2024 •
edited
Loading

Resolves: #2674

Added small, medium and large scales for Figma

Remaining work

  • Fontsize should follow size
  • Are we happy with these values as default?

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Nov 19, 2024 •
edited
Loading

⚠️ No Changeset found

Latest commit: cc807fd

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

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 19, 2024 •
edited
Loading

Preview deployments for this pull request:

Storybook - 20. Dec 2024 - 15:28

Storefront - 20. Dec 2024 - 15:29

@mimarz mimarz changed the title Add typography based sizing feat(tokens): add typography based sizing Nov 21, 2024
@mimarz mimarz marked this pull request as draft November 22, 2024 08:40
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 27, 2024 •
edited
Loading

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 55.2% 3198 / 5793
🔵 Statements 55.2% 3198 / 5793
🔵 Functions 86.6% 181 / 209
🔵 Branches 77.07% 521 / 676
File CoverageNo changed files found.
Generated in workflow #1388 for commit cc807fd by the Vitest Coverage Report Action

Febakke and others added 21 commits December 19, 2024 12:26
@Febakke @mimarz
Setting up utility tokens
2600956
@Febakke @mimarz
Created new scale and connected to the semantic layer
8354f11
@Febakke @mimarz
Added small and large. Added themes
1d489d6
@Febakke @mimarz
Small changes
36af6d0
@Febakke @mimarz
Changec lg size 20 -> 21
4f8ecad
@Febakke @mimarz
Added different font-sizes to size modes
6358d3e
@Febakke @mimarz
Added calculation collection
adada01
@mimarz
wip
1353a25
@Febakke @mimarz
moved utility -> size
8ea8afb 
renamed calculation -> global
Created alias for typography tokens
@Febakke @mimarz
Reversed _ naming
6aad95a
@mimarz
floor conversion
d70158b
@Febakke @mimarz
Static typography values
bcbacdc
@Febakke @mimarz
Fixed medium and global sizing scale
b47aa37
@mimarz
tweak round
31436f3
@mimarz
tweakin filtering
346c4ac
@Febakke @mimarz
Fixed wrong alias tokens
67ab5eb
@Febakke @mimarz
Fixed a small blunder in the sizing scale
cc171c0
@Febakke @mimarz
Moved typography sizing into typography
4954e7e 
reconnected global size to sm, md and lg
expanded themes to include new typography size sets
@Febakke @mimarz
Wrong mode_font_size on md
2a7afbb
@Febakke @mimarz
Used "_" naming on sm, md and lg instead of global
b36f421
@Febakke @mimarz
Fixed wrong references
b1f2e0e
@mimarz mimarz force-pushed the add-typography-based-sizing branch from ce430b8 to b1f2e0e Compare December 19, 2024 11:29
@mimarz mimarz marked this pull request as ready for review December 19, 2024 14:14
@mimarz mimarz marked this pull request as draft December 20, 2024 07:44
@mimarz mimarz marked this pull request as ready for review December 20, 2024 09:23
unekinn
unekinn requested changes Dec 20, 2024
View reviewed changes
Copy link
Contributor

@unekinn unekinn left a comment

Choose a reason for hiding this comment

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

Nice work! I found a few things that could be issues.

packages/css/src/index.css Outdated Show resolved Hide resolved
packages/theme/brand/altinn/semantic.css Outdated Show resolved Hide resolved
packages/theme/brand/altinn/typography/primary.css Outdated Show resolved Hide resolved
apps/storefront/tokens/altinn/dark.ts
@@ -2433,12 +2433,12 @@ export const dimension = [
},
{
$type: "dimension",
$value: "0rem",
$value: "NaN",
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these values used anywhere? If they are, I think everything becoming NaN would be a problem 😄

Copy link
Collaborator

Choose a reason for hiding this comment

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

blæææ

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure how to fix this in the short term 🤔 Got any ideas?

packages/cli/src/tokens/build/formats/css.ts Show resolved Hide resolved
packages/cli/src/tokens/build/transformers.ts Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@unekinn unekinn unekinn requested changes

@mimarz mimarz Awaiting requested review from mimarz mimarz is a code owner

@Barsnes Barsnes Awaiting requested review from Barsnes Barsnes is a code owner

@eirikbacker eirikbacker Awaiting requested review from eirikbacker eirikbacker is a code owner

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update sizing design-tokens with font-based calculation
3 participants
@Febakke @unekinn @mimarz