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(select-modal): add ionic theme styling #30007

Merged
merged 6 commits into from
Nov 8, 2024
Merged

feat(select-modal): add ionic theme styling #30007

merged 6 commits into from
Nov 8, 2024

Conversation

thetaPC
Copy link
Contributor

@thetaPC thetaPC commented Nov 7, 2024

Issue number: internal


What is the current behavior?

ion-select-modal does not have styles for ionic theme.

What is the new behavior?

  • Added styles for ionic theme
  • Added snapshots

No changes were made to the other interfaces. Those will continue to follow the current styles.

Does this introduce a breaking change?

  • Yes
  • No

Other information

Preview

Copy link

vercel bot commented Nov 7, 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 Nov 8, 2024 7:37pm

@github-actions github-actions bot added the package: core @ionic/core package label Nov 7, 2024
Comment on lines -79 to -82
:host(.ion-focused) .item-native {
// This prevents the focus ring from clipping.
overflow: visible;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No longer needed and it was interfering with the focus ring.

Comment on lines +98 to +104
--border-width: #{globals.$ion-border-size-0 globals.$ion-border-size-0 globals.$ion-border-size-025
globals.$ion-border-size-0};
}

:host(.item-lines-inset) {
--inner-border-width: #{globals.$ion-border-size-0} #{globals.$ion-border-size-0} #{globals.$ion-border-size-025} #{globals.$ion-border-size-0};
--inner-border-width: #{globals.$ion-border-size-0 globals.$ion-border-size-0 globals.$ion-border-size-025
globals.$ion-border-size-0};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just moved the variables to use only one #{}

Comment on lines +162 to +168
.label-text-wrapper {
text-overflow: ellipsis;

white-space: nowrap;

overflow: hidden;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Noticed that the select options weren't truncating with ellipsis. Moved this to common since every theme behaves the same.

@thetaPC thetaPC marked this pull request as ready for review November 7, 2024 22:21
@thetaPC thetaPC requested a review from a team as a code owner November 7, 2024 22:21
Copy link
Member

Choose a reason for hiding this comment

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

I believe the radios should be hidden for single-select (like we do for MD theme)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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!

@thetaPC thetaPC merged commit a1f3fcc into next Nov 8, 2024
46 checks passed
@thetaPC thetaPC deleted the ROU-11316 branch November 8, 2024 21:51
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.

2 participants