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

refactor(responsive): use JS module to create breakpoints lookup object #8051

Merged

Conversation

jcfranco
Copy link
Member

Related Issue: N/A

Summary

Stems from #8041 (comment).

This changes how breakpoints are built and used (responsive module):

  • uses design tokens JS output target to reference breakpoint values
  • prevents possible disconnect between stylesheet and conditional rendering breakpoint values (only the latter could be overridden via CSS props)
  • the breakpoints lookup object no longer needs to be obtained asynchronously

This also updates and unskips the responsive spec test.

Note: this includes an update to the testing Jest config to apply Stencil's transformer to the design token's headless ESM file as the default one was not transforming properly and leading to Jest encountered an unexpected token errors when running the spec test.

@jcfranco jcfranco requested a review from a team as a code owner October 25, 2023 06:38
@jcfranco jcfranco requested a review from alisonailea October 25, 2023 06:38
@jcfranco jcfranco added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Oct 25, 2023
@github-actions github-actions bot added the refactor Issues tied to code that needs to be significantly reworked. label Oct 25, 2023
@jcfranco jcfranco added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Oct 25, 2023
Copy link
Member

@driskull driskull left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

github-actions bot commented Nov 3, 2023

This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.

@github-actions github-actions bot added the Stale Issues or pull requests that have not had recent activity. label Nov 3, 2023
@jcfranco jcfranco removed the Stale Issues or pull requests that have not had recent activity. label Nov 10, 2023
@jcfranco
Copy link
Member Author

@alisonailea I'm going to proceed with merging, but please LMK if you spot anything and I can address afterwards.

@jcfranco jcfranco merged commit e7165cc into main Nov 10, 2023
13 of 15 checks passed
@jcfranco jcfranco deleted the jcfranco/update-responsive-util-to-use-esm-token-exports branch November 10, 2023 03:25
@github-actions github-actions bot added this to the 2023 November Priorities milestone Nov 10, 2023
benelan pushed a commit that referenced this pull request Nov 13, 2023
…ct (#8051)

**Related Issue:** N/A

## Summary

Stems from
#8041 (comment).

This changes how breakpoints are built and used (`responsive` module):

* uses design tokens JS output target to reference breakpoint values
* prevents possible disconnect between stylesheet and conditional
rendering breakpoint values (only the latter could be overridden via CSS
props)
* the breakpoints lookup object no longer needs to be obtained
asynchronously

This also updates and unskips the `responsive` spec test.

**Note**: this includes an update to the testing Jest config to apply
Stencil's transformer to the design token's headless ESM file as the
default one was not transforming properly and leading to `Jest
encountered an unexpected token` errors when running the spec test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr ready for visual snapshots Adding this label will run visual snapshot testing. refactor Issues tied to code that needs to be significantly reworked.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants