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

Feature/pxweb2 63 add global breakpoint tokens #74

Merged

Conversation

SjurSutterudSagen
Copy link
Contributor

Since we need to control the layout for different sized screens and such, we need the breakpoints for the media queries. This commit adds the breakpoints to a file, and adds that to the style-dictionary build command. The same as how we define and use the Spacings. In this way we have 1 place where all the breakpoint values are set, and can change them or add to them later if needed.

One thing to consider is that currently the breakpoint tokens are imported (@use) from the files that are generated in the ui lib, and not from inside the main app itself. I am not sure if we want this, or to create another set of files inside the app's own folder, and import from there. This is very easy to add in our current implementation, just add another output file to the build.js in the lib. And update the .gitignore if we don't want them committed to the repo.

Also added an example of the breakpoints working to the main app.

Since we need to control the layout for different sized screens and
such, we need the breakpoints for the media queries. This commit adds
the breakpoints to a file, and adds that to the style-dictionary
build command. The same as how we define and use the Spacings. In this
way we have 1 place where all the breakpoint values are set, and can
change them or add to them later if needed.
Since we want to see the breakpoints working, this commit adds an
example to the main app page.

One thing to consider is that currently the breakpoint tokens are
imported (@use) from the files that are generated in the ui lib, and
not from inside the app itself. I am not sure if we want this, or to
create another set of files inside the app's own folder, and import
from there. This is very easy to add in our current implementation,
just add another output file to the build.js in the lib.
@SjurSutterudSagen SjurSutterudSagen requested review from a team March 13, 2024 15:23
@SjurSutterudSagen SjurSutterudSagen marked this pull request as ready for review March 13, 2024 15:27
MikaelNordberg
MikaelNordberg previously approved these changes Mar 14, 2024
Copy link
Contributor

@MikaelNordberg MikaelNordberg left a comment

Choose a reason for hiding this comment

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

Looks good

Improves how the scss is imported. This commit adds the alias '$ui' to the
pxweb2 app's vite config, which can be used to import from the pxweb2-ui
lib in a cleaner way than traversing the filesystem with '../..'.

Co-authored-by: Michael Pande <>
Copy link

cloudflare-workers-and-pages bot commented Mar 14, 2024

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 888b7b3
Status: ✅  Deploy successful!
Preview URL: https://0af07605.pxweb2.pages.dev
Branch Preview URL: https://feature-pxweb2-63-add-global.pxweb2.pages.dev

View logs

MikaelNordberg
MikaelNordberg previously approved these changes Mar 14, 2024
Copy link
Contributor

@MikaelNordberg MikaelNordberg left a comment

Choose a reason for hiding this comment

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

Looks good

Copy link
Contributor

@MikaelNordberg MikaelNordberg left a comment

Choose a reason for hiding this comment

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

Looks good

@SjurSutterudSagen SjurSutterudSagen merged commit b1276fc into main Mar 14, 2024
5 checks passed
@SjurSutterudSagen SjurSutterudSagen deleted the feature/PXWEB2-63-add-global-breakpoint-tokens branch March 14, 2024 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants