Skip to content

feat(toast): allow custom positioning relative to specific element #28248

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

Merged
merged 31 commits into from
Oct 4, 2023

Conversation

averyjohnston
Copy link
Contributor

@averyjohnston averyjohnston commented Sep 27, 2023

Issue number: resolves #17499


What is the current behavior?

Currently, there isn't a way to position toasts such that they don't overlap navigation elements such as headers, footers, and FABs.

What is the new behavior?

Added the new positionAnchor property, which specifies an element that the toast's position should be anchored to.

While the name can be tweaked, we should take care to keep the relation between it and the position property clear. The position acts as a sort of "origin" point, and the toast is moved from there to sit near the chosen anchor element. This is important because it helps clarify why the toast sits above the anchor for position="bottom" and vice versa.

I chose not to rename the position prop itself to avoid breaking changes.

Docs PR: ionic-team/ionic-docs#3158

Does this introduce a breaking change?

  • Yes
  • No

Other information

@github-actions github-actions bot added package: core @ionic/core package package: angular @ionic/angular package labels Sep 27, 2023
@github-actions github-actions bot added the package: vue @ionic/vue package label Sep 28, 2023
}

/**
* We don't include safe area here because that should already be
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional: We could move this to a utility function so both iOS/MD animations can use it. However, since it's only used twice this is optional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I decided against this because the two are subtly different -- they have different starting offsets and, more importantly, the bottom positions are generally calculated using addition for md but subtraction for ios. This is because md uses style.bottom, meaning the position is calculated from the bottom edge of the screen, but ios uses translateY which calculates from the top edge instead.

I could certainly have the util function check the mode and act accordingly, but the context behind the difference would sit outside the util, making it harder to follow the code. There's probably some refactoring that could be done here to align the two modes, but that feels outside the scope of this PR.

Copy link
Contributor Author

@averyjohnston averyjohnston Oct 2, 2023

Choose a reason for hiding this comment

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

Although, if I need to avoid implicit keyframes in the leave animation, in theory it might make more sense to do this calculation out in toast.tsx so I can pass it to both the enter and leave animations 🤔 Hmm. I'll play around with it some.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've gone with the above idea. Let me know if you think it'd be worth it to make a tech debt ticket to sync up how the toast's position is calculated between modes, so the new getAnimationPosition util can be cleaned up a little. I'm not sure if it's worth the effort since the position-related CSS would need touching too 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

I like your approach here. The additional benefit is if devs want to create a custom toast animation they can still take advantage of the anchor/position information since we pass that in.

}

/**
* We don't include safe area here because that should already be
Copy link
Contributor

Choose a reason for hiding this comment

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

I like your approach here. The additional benefit is if devs want to create a custom toast animation they can still take advantage of the anchor/position information since we pass that in.

@liamdebeasi
Copy link
Contributor

I reverted a flaky screenshot diff in 5ded707, so make sure you pull latest

Copy link
Contributor

@liamdebeasi liamdebeasi left a comment

Choose a reason for hiding this comment

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

Great work! I'm fine with the naming convention.

Copy link
Contributor

@sean-perkins sean-perkins left a comment

Choose a reason for hiding this comment

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

This feature works great on device 🎉 this is such a great improvement for the user experience!

@averyjohnston averyjohnston merged commit 897ff6f into feature-7.5 Oct 4, 2023
@averyjohnston averyjohnston deleted the FW-4520 branch October 4, 2023 19:06
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: vue @ionic/vue package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants