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): does not match MD spec for tablet dimensions #27462

Closed
wants to merge 1 commit into from

Conversation

GlenOttley
Copy link
Contributor

@GlenOttley GlenOttley commented May 12, 2023

closes #23977

Issue number: #23977


What is the current behavior?

All ion-alerts in md mode have a max width of 280px, and the alert content has a max height of 240px.

What is the new behavior?

ion-alert max width is now 560px, per MD spec

Does this introduce a breaking change?

  • Yes
  • No

Other information

@GlenOttley GlenOttley requested a review from a team as a code owner May 12, 2023 12:44
@stackblitz
Copy link

stackblitz bot commented May 12, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@github-actions github-actions bot added the package: core @ionic/core package label May 12, 2023
@liamdebeasi
Copy link
Contributor

Thank you for the PR and apologies for the delay. I created a newer version of this PR using your changes as a base: #28501. I gave you co-author credit in this new PR as well.

I am going to close this in favor of the linked PR. Let me know if you have any questions.

github-merge-queue bot pushed a commit that referenced this pull request Nov 16, 2023
Issue number: resolves #23977

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->

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

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- 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
- [x] No

<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

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

---------

Co-authored-by: GlenOttley <[email protected]>
Co-authored-by: ionitron <[email protected]>
Co-authored-by: Brandy Carney <[email protected]>
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
2 participants