Skip to content

test(spinner): add transform test back#31017

Merged
brandyscarney merged 4 commits intomainfrom
spinner-transform-test
Mar 20, 2026
Merged

test(spinner): add transform test back#31017
brandyscarney merged 4 commits intomainfrom
spinner-transform-test

Conversation

@thetaPC
Copy link
Copy Markdown
Contributor

@thetaPC thetaPC commented Mar 18, 2026

Issue number: N/A


What is the current behavior?

While working on the migration for the spinner to Ionic Modular, I noticed that we had a Transform test page without any context of why it was there. I found out that it's meant for a bug that was reported in v4 and we did have a test for it at some point but it was removed at a later date because it was discovered that it wasn't doing anything.

So we don't have any coverage of it if there's a regression.

What is the new behavior?

  • Added a test to prevent a regression

Does this introduce a breaking change?

  • Yes
  • No

Other information

How to test:

  1. Be on your local
  2. Navigate to spinner.scss
  3. Update the following code snippet:
- :host(.spinner-circular) svg {
+ :host(.spinner-circular) {
  animation: spinner-circular linear infinite;
}
  1. By making this code change, we are introducing the original issue.
  2. Run the test
  3. Notice that it fails
  4. Undo the code change
  5. Run the test
  6. Verify that it passes

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 18, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
ionic-framework Ready Ready Preview, Comment Mar 18, 2026 8:54pm

Request Review

@thetaPC thetaPC marked this pull request as ready for review March 18, 2026 21:13
@thetaPC thetaPC requested a review from a team as a code owner March 18, 2026 21:13
@thetaPC thetaPC requested review from gnbm and removed request for gnbm March 18, 2026 21:13
Copy link
Copy Markdown
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.

Tested locally. Nice catch!

@thetaPC thetaPC added this pull request to the merge queue Mar 19, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Mar 19, 2026
@thetaPC thetaPC added this pull request to the merge queue Mar 19, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 19, 2026
@brandyscarney brandyscarney added this pull request to the merge queue Mar 20, 2026
Merged via the queue into main with commit 5657e7d Mar 20, 2026
52 checks passed
@brandyscarney brandyscarney deleted the spinner-transform-test branch March 20, 2026 17:09
os-davidlourenco pushed a commit that referenced this pull request Mar 26, 2026
Issue number: N/A

---------

<!-- 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. -->

While working on the migration for the spinner to Ionic Modular, I
noticed that we had a [Transform test
page](https://github.com/ionic-team/ionic-framework/blob/2b5b9137fc164c2f3305e493510a884c0afbfcf0/core/src/components/spinner/test/transform/index.html#L5)
without any context of why it was there. I found out that it's meant for
a [bug](#19247) that
was reported in v4 and we did have a
[test](https://github.com/ionic-team/ionic-framework/pull/24643/changes#diff-7b7ff84d3845fbde015775aa2da960310e80f79ec01b1f4a5957d751eddce7c9R1)
for it at some point but it was removed at a later date because it was
[discovered](#25259)
that it wasn't doing anything.

So we don't have any coverage of it if there's a regression.

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

- Added a test to prevent a regression

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!--
  If this introduces a breaking change:
1. Describe the impact and migration path for existing applications
below.
  2. Update the BREAKING.md file with the breaking change.
3. Add "BREAKING CHANGE: [...]" to the commit description when merging.
See
https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer
for more information.
-->


## Other information

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

How to test:
1. Be on your local
2. Navigate to `spinner.scss`
3. Update the following code snippet:
```diff
- :host(.spinner-circular) svg {
+ :host(.spinner-circular) {
  animation: spinner-circular linear infinite;
}
```
4. By making this code change, we are introducing the [original
issue](https://github.com/ionic-team/ionic-framework/pull/24643/changes#diff-fa8f6fb72eceb39e2482c0dbc083f69ecdabd411be541c21947f8e8e9bf9ee48L118).
5. Run the test
6. Notice that it fails
7. Undo the code change
8. Run the test
9. Verify that it passes
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