Skip to content

fix: TextInput loses focus inside Banner on iOS only - #4179

Open
abdulbasithqb wants to merge 4 commits into
callstack:5.0from
qburst:fixes/banner-focus-fix
Open

fix: TextInput loses focus inside Banner on iOS only#4179
abdulbasithqb wants to merge 4 commits into
callstack:5.0from
qburst:fixes/banner-focus-fix

Conversation

@abdulbasithqb

Copy link
Copy Markdown
Contributor

Motivation

TextInput loses focus every type a character is entered and keyboard is dismissed. This happens only on iOS real device.
On android real device it works as expected.

Related issue

issue #4178

Result

1. Banner with TextInput 2. Banner with Text

@abdulbasithqbabdulbasithqb changed the title Fixes/banner focus fixfix: TextInput loses focus inside Banner on iOS onlyNov 13, 2023
@callstack-bot

callstack-bot commented Nov 13, 2023

Copy link
Copy Markdown

Hey @abdulbasithqb, thank you for your pull request 🤗. The documentation from this branch can be viewed here.

Comment threadsrc/components/Banner.tsx Outdated
@lukewalczaklukewalczak linked an issue Nov 28, 2023 that may be closed by this pull request
@lukewalczaklukewalczak added this to the 5.12.0 milestone Jan 8, 2024
lukewalczak
lukewalczak previously approved these changes Jan 8, 2024
@lukewalczak
lukewalczak dismissed their stale reviewJanuary 10, 2024 20:38

It may break a layout for users who pass a component as a child into the Banner component

@lukewalczak

Copy link
Copy Markdown
Member

It may break a layout for users who pass a <Text /> component as a child into the Banner component:

 <Banner
onLayout={handleLayout}
actions={[
{
label: `Set ${useCustomTheme ? 'default' : 'custom'} theme`,
onPress: () => setUseCustomTheme(!useCustomTheme),
},
{
label: 'Fix it',
onPress: () => setVisible(false),
},
]}
icon={require('../../assets/images/email-icon.png')}
visible={visible}
onShowAnimationFinished={() =>
console.log('Completed opening animation')
}
onHideAnimationFinished={() =>
console.log('Completed closing animation')
}
theme={useCustomTheme ? customTheme : defaultTheme}
style={styles.banner}
>
<Text>
Two line text string with two actions. One to two lines is preferable
on mobile.
</Text>
</Banner>
beforeafter
Zrzut ekranu 2024-01-10 o 21 39 40Zrzut ekranu 2024-01-10 o 21 39 24

@abdulbasithqb

Copy link
Copy Markdown
ContributorAuthor

It may break a layout for users who pass a <Text /> component as a child into the Banner component:

 <Banner
onLayout={handleLayout}
actions={[
{
label: `Set ${useCustomTheme ? 'default' : 'custom'} theme`,
onPress: () => setUseCustomTheme(!useCustomTheme),
},
{
label: 'Fix it',
onPress: () => setVisible(false),
},
]}
icon={require('../../assets/images/email-icon.png')}
visible={visible}
onShowAnimationFinished={() =>
console.log('Completed opening animation')
}
onHideAnimationFinished={() =>
console.log('Completed closing animation')
}
theme={useCustomTheme ? customTheme : defaultTheme}
style={styles.banner}
>
<Text>
Two line text string with two actions. One to two lines is preferable
on mobile.
</Text>
</Banner>

before after
Zrzut ekranu 2024-01-10 o 21 39 40Zrzut ekranu 2024-01-10 o 21 39 24

any suggestion to resolve this issue ?

@seb-zabielski

seb-zabielski commented Jun 4, 2024

Copy link
Copy Markdown
Contributor

The problem with broken layout if we pass <Text> as child can be fixed by adding style to the <View> same as <Text> has: style={styles.message}.

There are other potential cases where changes from this pull request might cause error for the users that already using Banner component:

  1. More than one string/Text child (children as array):
<Bannervisible={visible}>
first child
<Text>second child</Text></Banner>
  1. Text wrapped in React.Fragment
<Bannervisible={visible}><>some text</></Banner>

Potential solutions that come to mind are either to check each child and wrap the strings in Text, or to treat this as a breaking change, but then it would not be a fix, but a major change

@lukewalczak
lukewalczakforce-pushed the main branch 6 times, most recently from 1062366 to 1da5d15CompareMay 8, 2025 09:47
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TextInput loses focus inside Banner on iOS only

4 participants

@abdulbasithqb@callstack-bot@lukewalczak@seb-zabielski