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/add conatiner queries #225

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

jackmcpickle
Copy link
Collaborator

Addresses #223 by adding container mixin

Currently some issue with sass-true and container mixin. Maybe a bug?

@jackmcpickle
Copy link
Collaborator Author

@andreimoment Review?

@jackmcpickle jackmcpickle force-pushed the feature/add-conatiner-queries branch from 26c5dec to 59903cf Compare February 27, 2023 11:32
@andreimoment
Copy link

@jackmcpickle thank you for submitting these. I noticed a few things:

  • I couldn't run the tests - adding @container to the tests results in errors - I see you've commented out that line.
  • When the @container tests run OK, I recommend:
    • adding tests using the new container units
    • adding a test for the "named: " parameter
  • I did not see any additions pertaining to the "named: ..." parameter.

Note, I am not a maintainer for this - just someone who suggested adding the feature. I apologize for not acting on this sooner - SCSS is not my forte and it will take me a bit of time to understand all of the code and propose an update.

Thank you for acting swiftly and moving this in the right direction.

@jackmcpickle
Copy link
Collaborator Author

@andreimoment all good. I thought I'd just start with the unnamed @container queries for now.

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.

3 participants