Issue number: internal
---------
<!-- 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?
The `ion-button` component is used in several tests to navigate or show
overlays. This causes screenshot diffs in unrelated tests any time the
UI of the `ion-button` is updated.
## What is the new behavior?
Removes the `ion-button` elements from unrelated tests.
Did not remove the `ion-button`s from the following tests:
- All `ion-button`s in an `ion-buttons` component
- An `ion-button` inside of a menu
- breadcrumbs/test/basic (uses a clear button in a list header, needs to
be moved)
- input/test/slot
- item/test/buttons
- item/test/colors
- item/test/dividers
- item/test/inputs
- item/test/media
- list-header/test/basic
- ripple-effect/test/basic
- router/test/basic
- router/test/guards
- router-outlet/test/basic
- select/test/slot
- textarea/test/slot
Updates the icon/basic test to use the right icon names by comparing
against the v3 names: https://ionicframework.com/docs/v3/ionicons/
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
---------
Co-authored-by: ionitron <hi@ionicframework.com>
Issue number: N/A
---------
## What is the current behavior?
Currently, every Sass variable in Ionic has the `!default` flag added to
the end.
From the [Sass variables
documentation](https://sass-lang.com/documentation/variables/):
> Normally when you assign a value to a variable, if that variable
already had a value, its old value is overwritten. But if you’re writing
a Sass library, you might want to allow your users to configure your
library’s variables before you use them to generate CSS.
>
> To make this possible, Sass provides the `!default` flag. This assigns
a value to a variable only if that variable isn’t defined or its value
is [null](https://sass-lang.com/documentation/values/null). Otherwise,
the existing value will be used.
In past versions of Ionic Framework, developers wrote Sass variables to
rebuild Ionic Framework using their own values. In the latest versions
of Ionic Framework, this is not possible.
## What is the new behavior?
Removes the `!default` flag from all Sass variables.
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
-------
Co-authored-by: brandyscarney <brandyscarney@users.noreply.github.com>
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. -->
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Migrates the test infrastructure to use `palettes` instead of `themes`
## 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/.github/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. -->
This **does not** change where the light/dark/high contrast styles live
or how they are consumed in the test infra. That work is done here:
https://github.com/ionic-team/ionic-framework/pull/29149
Co-authored-by: Liam DeBeasi <liamdebeasi@users.noreply.github.com>
⚠️ This is a combination of previously approved PRs with the
exception of
fe9dca513c.
This change was made as a result of
https://github.com/ionic-team/ionic-framework-design-documents/pull/248.
Issue number: Internal
---------
<!-- 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. -->
Users do not have a way of increasing the contrast in Ionic apps. This
is valuable for people with low vision as increasing the contrast
between foreground and background content helps improve readability.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Adds a high contrast light and high contrast dark theme. As with our
other themes, developers can choose between system, class, and always
stylesheets.
While we aim to improve contrast for text and UI components, this
feature prioritizes text in the event that both text and UI component
cannot be improved without one negatively impacting the other.
## 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/.github/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. -->
Dev build: `7.6.7-dev.11706894781.1cd59fde`
Testing instructions:
1. Open `src/themes/test/css-variables`. Activate the high contrast
light and dark themes to verify that contrast does increase.
2. Use the dev build to integrate the theme into a test app (conference
app, starter app, etc).
I'd recommend using these imports:
```css
@import "@ionic/angular/css/themes/high-contrast.system.css";
@import "@ionic/angular/css/themes/high-contrast-dark.system.css";
```
Note: Make sure this is imported **after** `core.scss`
---------
Co-authored-by: Shawn Taylor <shawn@ionic.io>
Co-authored-by: Brandy Carney <brandyscarney@users.noreply.github.com>
Co-authored-by: Sean Perkins <13732623+sean-perkins@users.noreply.github.com>
Co-authored-by: ionitron <hi@ionicframework.com>
Co-authored-by: Maria Hutt <thetaPC@users.noreply.github.com>
Co-authored-by: Amanda Johnston <90629384+amandaejohnston@users.noreply.github.com>
In https://github.com/ionic-team/ionic-framework/issues/28981 there was
some confusion surrounding how to remove an overlay from the DOM if it
was never presented. The `dismiss` method will remove the overlay from
the DOM, but only if the overlay is visible. Otherwise, it's a no-op.
This PR updates the `dismiss` method docs for each overlay component to
note that developers can use the browser's remove method to remove the
element from the DOM.
Issue number: resolves#28541
---------
<!-- 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. -->
On iOS when dismissing the `ion-action-sheet`, the animation does not
account for the safe area of the device. This results in the action
sheet not completely animating off the visible viewport on a device with
safe area enabled.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- The `ion-action-sheet` will animate consistently off the viewport when
dismissing, including the safe area.
To better support custom animations not needing to account for the safe
area, the safe area has been added to the padding of the action sheet
container. This results in the height increasing based on the bottom
safe area and animating correctly when translating between [`100%` and
`0%`](9d57758e3e/core/src/components/action-sheet/animations/ios.enter.ts (L23)).
|Before|After|
|---|---|
|<video
src="https://github.com/ionic-team/ionic-framework/assets/13732623/d9475a54-f1b9-4283-ae1a-86e2b71a9d9e"></video>|<video
src="https://github.com/ionic-team/ionic-framework/assets/13732623/d051b4d9-4f45-4a51-b522-510f8cddef74"></video>|
In the recorded examples the bottom safe area is exaggerated to show the
dismiss animation differences.
## 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/.github/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. -->
To test this PR, I would recommend adding the following styles to the
action sheet basic test:
```html
<style>
ion-action-sheet {
--ion-safe-area-bottom: 100px;
}
</style>
```
You can then open dev-tools and slow the animation speed to 10%:
- Open and dismiss the action sheet tied to the "Basic" button.
- Verify that the action sheet animates completely off the view when
dismissed.
- Open the and dismiss the action sheet tied to the "Custom CSS Class"
button. This implementation customizes the `--height` of the action
sheet.
- Verify that the action sheet animates completely off the view when
dismissed.
---------
Co-authored-by: ionitron <hi@ionicframework.com>
Co-authored-by: Liam DeBeasi <liamdebeasi@users.noreply.github.com>
Issue number: resolves#27965
---------
https://www.youtube.com/watch?v=a_m7jxrTlaw&list=PLIvujZeVDLMx040-j1W4WFs1BxuTGdI_b&index=3
<!-- 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. -->
There is a lack of autocomplete support for AlertButton attribute of
`role` (there may be similar situations for other components too,
however, I was working on this component and found it).
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Support for autocomplete for the two types defined `cancel` and
`destructive`, and also supports any arbitrary string if passed in.
## 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. -->
I suggest there to be some global interface similar to the following:
```
interface LooseAutocomplete<T extends string> = T | Omit<string, T>;
```
I referenced this great [video](https://youtu.be/a_m7jxrTlaw) from Matt
@mattpocock
---------
Co-authored-by: Liam DeBeasi <liamdebeasi@users.noreply.github.com>
Issue number: internal
---------
<!-- 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. -->
`ion-button` is being used as a way to open the alerts. It's not being
used to test functionality.
When `ion-button` is updated then the screenshots in the alert tests
must be updated. This shouldn't happen when `ion-button` isn't necessary
to have in these tests.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Replaced `ion-button` with `button`
## 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/.github/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. -->
N/A
---------
Co-authored-by: ionitron <hi@ionicframework.com>
Issue number: Internal
---------
<!-- 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 team would like to ensure that Ionic Framework components that use
an Ionic color (primary, secondary, etc) on top of a contrast color pass
minimum contrast ratios as defined in the WCAG.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Introduces a revised set of Ionic colors that pass AA color contrast
guidelines when with the appropriate contrast.
## 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/.github/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. -->
---------
Co-authored-by: Sean Perkins <13732623+sean-perkins@users.noreply.github.com>
Co-authored-by: Brandy Carney <brandy@ionic.io>
Co-authored-by: Sean Perkins <sean@ionic.io>
Co-authored-by: Shawn Taylor <shawn@ionic.io>
Co-authored-by: Brandy Carney <brandyscarney@users.noreply.github.com>
Co-authored-by: Amanda Johnston <90629384+amandaejohnston@users.noreply.github.com>
Co-authored-by: Maria Hutt <thetaPC@users.noreply.github.com>
Co-authored-by: ionitron <hi@ionicframework.com>
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. -->
Action sheet buttons cannot be disabled. This behavior exists in iOS 17.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Action sheet buttons can be disabled
## 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/.github/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. -->
---------
Co-authored-by: ionitron <hi@ionicframework.com>
Issue number: resolves#28524
---------
<!-- 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. -->
Watchers in Stencil are constructed sometime between `connectedCallback`
and `componentDidLoad`. If a property is set/changed during that time it
is possible for the callback associated with the watcher to not fire
because the watcher has not been setup yet. This is most often with
`dist-custom-elements` and frameworks such as Angular when using a
binding (i.e. `[trigger]` instead of `trigger`)
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- The trigger callback associated with the watcher is manually called in
`componentDidLoad` for each overlay.
## 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. -->
Dev build: `7.5.5-dev.11699974376.13a15397`
Note: This is a timing related bug due to a behavior in Stencil, so I
did not write automated tests. However, I manually verified that this
issue a) reproduces on `main` and b) is fixed with this dev build for
each overlay component.
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. -->
Type checking inside of the Stencil unit tests have been disabled for a
long time. This has resulted in a difficult developer experience and
numerous issues (both types and implementation) within our unit tests.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Type checking is now enabled for all Stencil unit tests
- Tests have been updated to resolve type errors and implementation
errors
- Many `as any` casts were introduced, as many legacy tests test invalid
configurations of functions that require it (for example passing
`undefined` to an argument that cannot be `undefined`).
## 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. -->
To test this PR you can checkout the branch locally. Install
dependencies in the `/core` directory to make sure you are on at least
`@stencil/core@4.7.2`.
Opening either a `.spec.ts` or `.spec.tsx` file, validate that your IDE
detects types and can provide auto completions for jest global types.
If you desire, you can provide an invalid type and try building the
project - you will observe the build will fail due to the invalid type.
Issue number: resolves#18487
---------
<!-- 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. -->
Web-based users do not get a scrollbar when:
- alert has a long list of inputs (this also happens on `ion-select`
with the alert interface)
- `ion-select` uses the action-sheet interface and has a long list of
options
This makes it difficult for users to navigate through the options by
forcing them to use their keyboards. Some users may also not be used to
using their keyboards for navigation. Additionally, this can lead to
potential confusion that there are no other options.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
Web-based users get a scrollbar when:
- alert has a long list of inputs (this also happens on `ion-select`
with the alert interface)
- `ion-select` uses the action-sheet interface and has a long list of
options
## 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. -->
The issue was filed for the alert interface but it's also happening on
the action-sheet interface.
Dev build: 7.5.1-dev.11697570585.1774584d
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. -->
This project has several unused Sass variables still in the code base.
The team would like to remove these.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Removed unused Sass variables
## 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. -->
The original scope of this ticket was for checkbox only, but many other
components had unused sass variables, so I decided to tackle everything
all at once.
Since these variables are not used anywhere:
1. The build should pass
2. There should be no screenshot diffs
Issue number: resolves#24638, resolves#18592
---------
<!-- 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. -->
Developers have requested that Ionic Framework support the dynamic type
feature on iOS for accessibility purposes. Ionic applications do not
respond to font scaling on iOS which can create inaccessible
applications particularly for users with low vision. Ionic apps on
Android devices currently support the Android equivalent due to
functionality in the Chromium webview.
Developers have also requested a way of adjusting the fonts in their
Ionic UI components consistently.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Ionic components now use `rem` instead of `px` where appropriate. This
means devs can change the font size on `html` and the text in supported
Ionic components will scale up/down appropriately
- Add support for Dynamic Type on iOS (the iOS version of Dynamic Font
Scaling)
## 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. -->
---------
Co-authored-by: Maria Hutt <thetaPC@users.noreply.github.com>
Co-authored-by: Brandy Carney <brandyscarney@users.noreply.github.com>
Co-authored-by: Shawn Taylor <shawn@ionic.io>
Co-authored-by: ionitron <hi@ionicframework.com>
Co-authored-by: Sean Perkins <sean@ionic.io>
Co-authored-by: Sean Perkins <13732623+sean-perkins@users.noreply.github.com>
Co-authored-by: Amanda Johnston <90629384+amandaejohnston@users.noreply.github.com>
Issue number: resolves#27200
---------
<!-- 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. -->
A bug occurs when you click twice quickly to open an overlay with a
small timeout. In some cases, the overlay will present, dismiss,
present, then not dismiss the second time, getting stuck open. You can
reproduce manually this by grabbing the test HTML included in this PR
and putting it in a branch that doesn't include a fix.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- When an overlay with a short timeout is triggered twice quickly, it
will open-close-open-close.
- The behavior is the same for all overlay components
## 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. -->
Relevant links:
* https://github.com/ionic-team/ionic-framework/issues/27200
* https://ionic-cloud.atlassian.net/browse/FW-4374
* https://ionic-cloud.atlassian.net/browse/FW-4053
I'm not sure how to write an automated test for this bug due to the
short timeout required.
You can manually test the fix in [this
Stackblitz](https://stackblitz.com/edit/g1kjci?file=package.json) by
changing the Ionic version between 7.3.1 and
7.3.2-dev.11693262117.17edbf6d
---------
Co-authored-by: Liam DeBeasi <liamdebeasi@users.noreply.github.com>
Issue number: resolves#27928
---------
<!-- 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. -->
Action sheet, alert, picker, and toast did not have logic where the
overlay presents if `isOpen="true"` on load. Modal, popover, and loading
had this logic but did not have test coverage.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Updated action sheet, alert, picker, and toast to present if
`isOpen="true"` on load
- Added test coverage to all overlays for this functionality.
## 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. -->
Dev build: `7.2.3-dev.11691156718.1638345c`
Issue number: N/A
---------
## What is the current behavior?
Buttons containing only icons are not accessible as there is no way to
pass an `aria-label` attribute (or any other html attribute).
## What is the new behavior?
- Adds the `htmlAttributes` property on the `ActionSheetButton`
interface
- Passes the `htmlAttributes` to the buttons (both the buttons array and
the cancelButton)
- Adds two tests to verify `aria-label` and `aria-labelled-by` are
passed to a button with and without the cancel role - this was done
because action sheet breaks these buttons up when rendering
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
Issue number: N/A
---------
<!-- Please refer to our contributing documentation for any questions on
submitting a pull request, or let us know here if you need any help:
https://ionicframework.com/docs/building/contributing -->
<!-- Some docs updates need to be made in the `ionic-docs` repo, in a
separate PR. See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#modifying-documentation
for details. -->
<!-- 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. -->
I noticed some test slowdowns with the introduction of action sheet
tests, so I wanted to try and find a way to speed these tests up.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- htmlAttributes test was ported to a spec test
- the functionality/behavioral tests were being run per-mode when they
did not need to be, so I removed the per-mode checks
- Moved screenshot tests to a separate file so they can be better
paralellelized
## 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. -->
Issue number: Internal
---------
<!-- Please refer to our contributing documentation for any questions on
submitting a pull request, or let us know here if you need any help:
https://ionicframework.com/docs/building/contributing -->
<!-- Some docs updates need to be made in the `ionic-docs` repo, in a
separate PR. See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#modifying-documentation
for details. -->
<!-- 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 counter for incrementing the `id` and `z-index` of an overlay is
incremented whenever the `connectedCallback` is fired for an overlay.
When an overlay is presented and/or conditionally rendered, the overlay
`id` can increment by `n+2` instead of `n+1`.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Increments all overlay ids consistently
- Removes legacy `ion-modal-{id}` and `ion-popover-{id}` logic
- Adds unit tests for the id behavior
- Tests are split up into separate files so that the counter is always
starting from `0`
- Adds an integration test with the Angular test app to verify
conditional rendering behavior
## 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. -->
Issue number: N/A
---------
<!-- Please refer to our contributing documentation for any questions on
submitting a pull request, or let us know here if you need any help:
https://ionicframework.com/docs/building/contributing -->
<!-- Some docs updates need to be made in the `ionic-docs` repo, in a
separate PR. See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#modifying-documentation
for details. -->
<!-- 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. -->
This is the first PR to introduce the infrastructure required to add
test generators to the Ionic Framework project. This PR introduces the
file name changes necessary to support two playwright configs, so I
recommend reviewing the PR by commit:
1e5012cea1
- Created a `playwright.config-legacy.ts` file and updates
`package.json`.
- Running `npm run test.e2e` will run the generator tests, and running
`npm run test.e2e.legacy` will run the legacy tests.
4fe8de7df7
- Updates the GitHub Action scripts to run both the modern and legacy
E2E tests. I added command modifiers to avoid collisions with output
directories.
e8bcfaf926
- Updates `*.e2e.ts` files to have the legacy format name:
`*.e2e-legacy.ts`. This naming scheme is required for the two Playwright
configs to pull in the correct files. When migrating tests to
generators, team members will rename the file to remove the `-legacy`
part.
5bf196c36d
(warning: lots of files!)
- Updates the `*.e2e.ts-snapshots` directories to have the legacy format
name: `*.e2e-legacy.ts-snapshots`. The screenshot directory in
Playwright is generated based on the test file name which is why we are
updating the screenshot directory. When migrating tests to generators,
team members will rename the directory to remove the `-legacy` part.
## 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. -->
Issue number: N/A
---------
<!-- Please refer to our contributing documentation for any questions on
submitting a pull request, or let us know here if you need any help:
https://ionicframework.com/docs/building/contributing -->
<!-- Some docs updates need to be made in the `ionic-docs` repo, in a
separate PR. See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#modifying-documentation
for details. -->
<!-- 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. -->
`page.locator` is synchronous, but we were `await`ing the calls:
https://playwright.dev/docs/api/class-page#page-locator
We were also doing `page.locator(...).click()` when we can just do
`page.click([selector])` directly,
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Removes `await` usage from `page.locator`
- Removes `page.locator().click()` usage in favor of `page.click()`
## 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. -->
Issue number: N/A
---------
<!-- Please refer to our contributing documentation for any questions on
submitting a pull request, or let us know here if you need any help:
https://ionicframework.com/docs/building/contributing -->
<!-- Some docs updates need to be made in the `ionic-docs` repo, in a
separate PR. See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#modifying-documentation
for details. -->
<!-- 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 team decided on some new processes for adding tests. One of those
processes is that test directories should use kebab case.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Updates test directories to use kebab case
## 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. -->
<!-- Please refer to our contributing documentation for any questions on
submitting a pull request, or let us know here if you need any help:
https://ionicframework.com/docs/building/contributing -->
<!-- Some docs updates need to be made in the `ionic-docs` repo, in a
separate PR. See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#modifying-documentation
for details. -->
<!-- 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. -->
<!-- Issues are required for both bug fixes and features. -->
Issue URL: N/A
Playwright recommends using Locators with web-first assertions instead
of using `waitForSelector`:
https://playwright.dev/docs/api/class-frame#frame-wait-for-selector
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Removes waitForSelector usages for checking if components are visible
in favor of the `toBeVisible` or `toBeHidden` assertions.
## 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. -->
The Ionic Framework team would like to re-evaluate our approach to improving customization options when building mobile applications. We are looking at other solutions that a) make customizing an app UI easier and b) are applicable to a broader set of developers.
