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: 🐛 [IOSSDKBUG-482]iPad popover FilterFeedback list jumping #931

Merged
merged 5 commits into from
Dec 10, 2024

Conversation

restaurantt
Copy link
Contributor

Select/Deselect All cause view hight change.
On iPad, popover with UIKit set 'preferredContentSize'.
On iPhone, add animation to list.

@restaurantt restaurantt requested a review from a team as a code owner December 9, 2024 09:54
@restaurantt restaurantt requested review from billzhou0223 and removed request for a team December 9, 2024 09:54
Copy link
Contributor

@billzhou0223 billzhou0223 left a comment

Choose a reason for hiding this comment

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

Please add a video demonstrating the result.

Copy link
Contributor

@dyongxu dyongxu left a comment

Choose a reason for hiding this comment

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

The click event will be only triggered in the center of the bar item. It does not work in the area close to the border

@restaurantt
Copy link
Contributor Author

The click event will be only triggered in the center of the bar item. It does not work in the area close to the border

Fixed.

@restaurantt
Copy link
Contributor Author

Please add a video demonstrating the result.

Simulator.Screen.Recording.-.iPad.Pro.13-inch.M4.-.2024-12-10.at.16.34.33.mp4
Simulator.Screen.Recording.-.iPhone.16.Pro.Max.-.2024-12-10.at.16.50.58.mov
  1. iPad: popover has animation now. Tried to add animation to list and apply button, but performance is not so good. Also tried add animation to height changing, not work.
  2. iPhone, when 'presentationDetents' use '.height', looks not so good. Other cases are ok.

@dyongxu
Copy link
Contributor

dyongxu commented Dec 10, 2024

@restaurantt Since this looks better than before, I have approved it for this PR. We can continue to look into the improvement.

@billzhou0223 billzhou0223 merged commit d2a244f into SAP:main Dec 10, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants