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

feat(css): add CSS utility classes to the ionic theme bundle to match other themes #29974

Merged
merged 9 commits into from
Oct 30, 2024

Conversation

brandyscarney
Copy link
Member

@brandyscarney brandyscarney commented Oct 28, 2024

Issue number: internal


What is the current behavior?

The ionic theme includes the default (iOS/MD) bundle in the tests. Additionally, the ionic theme bundle is missing CSS utility classes provided by the default bundle, so removing that bundle causes issues in several tests.

What is the new behavior?

The default bundle is removed from the tests for the ionic theme because it adds global component styles that the ionic theme does not need. The missing utility files are imported, and padding/margin classes are generated from the design tokens, as many tests rely on ion-padding and ion-text-center being available. This update ensures the ionic theme includes the same classes offered in our documentation: https://ionicframework.com/docs/layout/css-utilities.

Does this introduce a breaking change?

  • Yes
  • No

Copy link

vercel bot commented Oct 28, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ionic-framework ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 29, 2024 8:02pm

Copy link
Member Author

Choose a reason for hiding this comment

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

This file includes the CSS utilities documented here: https://ionicframework.com/docs/layout/css-utilities

core/src/css/ionic/utils.bundle.ionic.scss Outdated Show resolved Hide resolved
core/src/css/ionic/utils.bundle.ionic.scss Outdated Show resolved Hide resolved
Copy link
Member

@tanner-reits tanner-reits 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 this is g2g

Comment on lines +146 to +147
const defaultSpaceUtilityClasses = generateDefaultSpaceUtilityClasses();
otherUtilityClasses.push(defaultSpaceUtilityClasses);
Copy link
Member Author

Choose a reason for hiding this comment

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

These were the only lines added - the rest is formatting

@brandyscarney brandyscarney changed the title chore(testing): remove the default ionic bundle for the ionic theme feat(css): add padding and margin classes to ionic theme to match other themes Oct 29, 2024
@brandyscarney brandyscarney changed the title feat(css): add padding and margin classes to ionic theme to match other themes feat(css): add CSS utility classes to the ionic theme bundle to match other themes Oct 29, 2024
@brandyscarney
Copy link
Member Author

Requesting re-review because I changed the scope of this PR.

@BenOsodrac
Copy link
Contributor

LGTM 🚀

Copy link
Member

@tanner-reits tanner-reits left a comment

Choose a reason for hiding this comment

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

LGTM

@brandyscarney brandyscarney merged commit 3306d71 into next Oct 30, 2024
49 checks passed
@brandyscarney brandyscarney deleted the bc/test-remove-default-bundle branch October 30, 2024 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core @ionic/core package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants