Skip to content

Conversation

thetaPC
Copy link
Contributor

@thetaPC thetaPC commented Oct 16, 2024

v8.3.3

brandyscarney and others added 5 commits October 2, 2024 13:41
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. -->

Test styles causes native buttons to have [certain
styling](https://github.com/ionic-team/ionic-framework/blob/main/core/scripts/testing/styles.css#L52-L64).
This was done to spruce up the buttons used for testing purposes only.
However, this ended up adding styles to native buttons within Ionic
components.

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

- Test styles for native buttons are only applied to buttons that are
not part of a Ionic component

## 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. Run the project locally from the `main` branch
2.
[Comment](https://github.com/ionic-team/ionic-framework/blob/5d208e9daacad63294489e0b1bd6ec2b2645babd/core/src/components/searchbar/searchbar.md.scss#L91)
out `border: 0` from `.searchbar-clear-button` from the searchbar `md`
theme file (`ios` also works)
3. Navigate to the basic test page:
`/src/components/searchbar/test/basic`
4. Notice a teal border around the clear buttons
5. Checkout to this PR's branch
6. Make sure steps 2-3 are done
7. Verify that the teal border is not being applied to the clear buttons
8. Verify that only native buttons outside of the Ionic components have
a teal appearance: `/src/components/loading/test/standalone` and
`/src/components/action-sheet/test/is-open`
Issue number: resolves #29885, resolves #29924

---------

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

React and Vue:

Tab bar could be a standalone element within `IonTabs` and would
navigate without issues with a router outlet before v8.3:

```tsx
<IonTabs>
  <IonRouterOutlet></IonRouterOutlet>

  <IonTabBar></IonTabBar>
</IonTabs>
```

It would work as if it was written as:

```tsx
<IonTabs>
  <IonRouterOutlet></IonRouterOutlet>

  <IonTabBar slot="bottom">
    <!-- Buttons -->
  </IonTabBar>
</IonTabs>
```

After v8.3, any `ion-tab-bar` that was not a direct child of `ion-tabs`
would lose it's expected behavior when used with a router outlet. If a
user clicked on a tab button, then the content would not be redirected
to that expected view.

React only:

Users can no longer add a `ref` to the `IonRouterOutlet`, it always
returns undefined.

```
<IonTabs>
      <IonRouterOutlet ref={ref}>

     <IonTabBar slot="bottom">
    <!-- Buttons -->
  </IonTabBar>
</IonTabs>
```

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

The fixes were already reviewed through PR
#29925 and PR
#29927. I split them
to make it easier to review.

React and Vue:

The React tabs has been updated to pass data to the tab bar through
context instead of passing it through a ref. By using a context, the
data will be available for the tab bar to use regardless of its level.

React only:

Reverted the logic for `routerOutletRef` and added a comment of the
importance of it.

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

N/A
@thetaPC thetaPC requested a review from a team as a code owner October 16, 2024 19:00
@thetaPC thetaPC requested review from tanner-reits and removed request for a team October 16, 2024 19:00
@github-actions github-actions bot added package: core @ionic/core package package: angular @ionic/angular package package: vue @ionic/vue package package: react @ionic/react package labels Oct 16, 2024
Copy link

vercel bot commented Oct 16, 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 Oct 16, 2024 7:02pm

Copy link
Contributor

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

G2G

@thetaPC thetaPC merged commit 99fa7b0 into 8.3.x Oct 16, 2024
92 checks passed
@thetaPC thetaPC deleted the release-8.3.3 branch October 16, 2024 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: angular @ionic/angular package package: core @ionic/core package package: react @ionic/react package package: vue @ionic/vue package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants