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

fix(alert): match MD spec on tablet #28501

Merged
merged 8 commits into from
Nov 16, 2023
Merged

fix(alert): match MD spec on tablet #28501

merged 8 commits into from
Nov 16, 2023

Conversation

liamdebeasi
Copy link
Contributor

@liamdebeasi liamdebeasi commented Nov 9, 2023

Issue number: resolves #23977


What is the current behavior?

The MD Alert on tablet dimensions does not match the MD spec

What is the new behavior?

  • MD Alert now follows the MD spec for tablet dimensions
  • Added tablet and mobile viewport mixins for alert and the card modal. (There should be no visual diffs for the card modal)

Does this introduce a breaking change?

  • Yes
  • No

Other information

This supersedes #27462 since I needed to add new screenshot tests. The author of that PR has been given co-author credit here.

@liamdebeasi liamdebeasi changed the title Fw 224 fix(alert): match MD spec on tablet Nov 9, 2023
@github-actions github-actions bot added the package: core @ionic/core package label Nov 9, 2023
Copy link
Contributor Author

@liamdebeasi liamdebeasi Nov 9, 2023

Choose a reason for hiding this comment

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

@liamdebeasi liamdebeasi marked this pull request as ready for review November 9, 2023 16:48
@liamdebeasi liamdebeasi requested review from a team, averyjohnston and brandyscarney and removed request for a team November 9, 2023 16:48
@thetaPC
Copy link
Contributor

thetaPC commented Nov 9, 2023

@liamdebeasi there's an open PR that also aims to fix the issue. Let's make sure that credit is given before merge.

@liamdebeasi
Copy link
Contributor Author

@thetaPC I gave them co-author credit in c7f5d52 which should translate to co-author credit in the squashed commit.

core/src/components/alert/alert.md.scss Outdated Show resolved Hide resolved
core/src/components/alert/alert.md.scss Outdated Show resolved Hide resolved
Copy link
Member

@brandyscarney brandyscarney left a comment

Choose a reason for hiding this comment

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

Changes look good to me! Just a small nit pick on the comment style. 😎

core/src/themes/ionic.mixins.scss Outdated Show resolved Hide resolved
core/src/themes/ionic.mixins.scss Outdated Show resolved Hide resolved
@liamdebeasi liamdebeasi added this pull request to the merge queue Nov 16, 2023
Merged via the queue into main with commit 6a2be9f Nov 16, 2023
46 checks passed
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.

bug: alert does not match MD spec for tablet dimensions
5 participants