Skip to content

fix(iOS): useHeaderHeight does not report correct value after cancelling search #2817

Merged
maciekstosio merged 5 commits intomainfrom
@maciekstosio/Fix-useHeaderHeight
Apr 1, 2025
Merged

fix(iOS): useHeaderHeight does not report correct value after cancelling search #2817
maciekstosio merged 5 commits intomainfrom
@maciekstosio/Fix-useHeaderHeight

Conversation

@maciekstosio
Copy link
Copy Markdown
Contributor

@maciekstosio maciekstosio commented Mar 31, 2025

Description

This PR fixes #2611.

After this changes the hook will provide header height values after focusing search bar.

Changes

It seems that we have a logic, that disables useHeaderHeight updates when the modal is being closed. The condition assumes that only if there is a presentedViewController, this is a modal. But it looks like it can also be a UISearchController, so we exclude that from the condition.

Screenshots / GIFs

Before After
before.mov
after.mov

Checklist

  • Included code example that can be used to test this change
  • Ensured that CI passes

@maciekstosio maciekstosio requested a review from kkafar March 31, 2025 10:32
Copy link
Copy Markdown
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

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

Left single comment.

Good job in catching this.

Comment thread ios/RNSScreenStack.mm Outdated
Comment on lines +78 to +81
BOOL isNotDismissingModal = screenController.presentedViewController == nil ||
(screenController.presentedViewController != nil &&
![screenController.presentedViewController isBeingDismissed]);
![screenController.presentedViewController isBeingDismissed]) ||
([screenController.presentedViewController isKindOfClass:[UISearchController class]]);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The condition is ok, but naming is misleading.

Either rename the variable to isNotDismissingModalOrPresentingSearchController or
split this condition - leave isNotDismissingModal as is and add new variable isNotPresentingSearchController and use it below.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmmm, I think the naming is correct, although it's unnecessarily (?) complicated.

BOOL isDismissingModal = (screenController.presentedViewController != nil && [screenController.presentedViewController isBeingDismissed]);

and then

if (![screenController hasNestedStack] && !isDismissingModal) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

changed in: 97e4b0d

@kkafar kkafar changed the title fix useHeaderHeight hook fix(iOS): useHeaderHeight does not report correct value after cancelling search Mar 31, 2025
Copy link
Copy Markdown
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

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

This one & we're cacy

Comment thread ios/RNSScreenStack.mm Outdated
Co-authored-by: Kacper Kafara <kacper.kafara@swmansion.com>
@maciekstosio maciekstosio merged commit aacef2e into main Apr 1, 2025
6 checks passed
@maciekstosio maciekstosio deleted the @maciekstosio/Fix-useHeaderHeight branch April 1, 2025 11:07
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.

useHeaderHeight does not update after cancelling search

2 participants