[HOLD for payment 2022-07-15] [$250] Emoji Search Bar focus indicator is missing - reported by Santhoshkumar Sellavel

[mvtglobally]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Open new Expensify in web/desktop
2. Open any chat
3. Click on the emoji icon in the composer
4. Enter the search input

Expected Result:

Search input has a focus indicator, like other search inputs

Actual Result:

Search input has no focus indicator.

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web
• Desktop App

Version Number: 1.1.76-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Expensify/Expensify Issue URL:

Screen.Recording.2022-05-21.at.2.48.45.AM.mov

Issue reported by: @Santhosh-Sellavel
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1653082016688519

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @MitchExpensify (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[Puneet-here]

Proposal

We can make a state isFocused and we can make it true at onFocus and false at onBlur

App/src/components/EmojiPicker/EmojiPickerMenu/index.js

Lines 437 to 442 in 6bac9af

	<Composer
	textAlignVertical="top"
	placeholder={this.props.translate('common.search')}
	placeholderTextColor={themeColors.textSupporting}
	onChangeText={this.filterEmojis}
	style={styles.textInput}

we need to use it like this:

onFocus={() => this.setState({isFocused: true})}
onBlur={() => this.setState({isFocused: false})}

now we have to use isFocused to change border color. We can do that by using the below line at composer

 style={[styles.textInput, this.state.isFocused && styles.borderColorFocus]}

[Harshdeepjoshi]

Proposal

we should use TextInput instead of Composer as Composer is intended to compose messages and should not be used in this place.

App/src/components/EmojiPicker/EmojiPickerMenu/index.js

Lines 437 to 442 in 6bac9af

	<Composer
	textAlignVertical="top"
	placeholder={this.props.translate('common.search')}
	placeholderTextColor={themeColors.textSupporting}
	onChangeText={this.filterEmojis}
	style={styles.textInput}

we should do

<TextInput 
textAlignVertical="top" 
placeholder={this.props.translate('common.search')} 
placeholderTextColor={themeColors.textSupporting} 
onChangeText={this.filterEmojis} 
style={styles.textInput}

Doing this would solve the issue but will alter the height of the input field slightly, but it would be the same as all the other input fields so I think this would not be a problem but, it could be changed if necessary.

[MitchExpensify]

This would be consistent but I'm not sure it adds much value, do you think this change is worth it in the name of consistency @michelle-thompson ?

[michelle-thompson]

Hmm, not sure what would be best here so adding Engineering label here!

[michelle-thompson]

Hmm, not sure what would be best here so adding Engineering label here!

[melvin-bot]

Triggered auto assignment to @TomatoToaster (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[TomatoToaster]

I think it's worth fixing this - it's definitely a small visual change but it feels like it would look better. I'm not on the management team so I'll let an engineer from there review the proposals and confirm this is worth it.

[melvin-bot]

Triggered auto assignment to @Christinadobrzyn (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[Christinadobrzyn]

Created job in Upwork

Internal - https://www.upwork.com/ab/applicants/1537544134022590464/job-details
External - https://www.upwork.com/jobs/~01d76621dc2b371865

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (Exported)

[melvin-bot]

Triggered auto assignment to @Luke9389 (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[Santhosh-Sellavel]

Open for new proposals.

[Harshdeepjoshi]

@Santhosh-Sellavel Please share your reviews on this proposal.

[liyamahendra]

Proposal

Problem

The Composer is not having the styles.borderColorFocus assigned to it.

Solution

• Add highlightComposer: false, to the state to keep track of whether the Composer is focused or not.
• Add onFocus and onBlur event handler to the composer to update the highlightComposer state variable.
• Set the style on the Composer based on the highlightComposer state variable.

Code changes:

this.state = {
    filteredEmojis: this.emojis,
    headerIndices: this.unfilteredHeaderIndices,
    highlightedIndex: -1,
    arePointerEventsDisabled: false,
    highlightComposer: false,
    selection: {
        start: 0,
        end: 0,
    },
};

<Composer
    textAlignVertical="top"
    placeholder={this.props.translate('common.search')}
    placeholderTextColor={themeColors.textSupporting}
    onChangeText={this.filterEmojis}
    onFocus={() => this.setState({highlightComposer: true})}
    onBlur={() => this.setState({highlightComposer: false})}
    style={[styles.textInput, this.state.highlightComposer && styles.borderColorFocus]}
    defaultValue=""
    ref={el => this.searchInput = el}
    autoFocus
    selectTextOnFocus={this.state.selectTextOnFocus}
    onSelectionChange={this.onSelectionChange}
/>

Screencast:

screencast.2022-06-18.19-20-35.mp4

[Puneet-here]

@Santhosh-Sellavel, can you review my proposal ?

[Luke9389]

@liyamahendra's proposal is just restating @Puneet-here's from above.
Also @Santhosh-Sellavel can you give a written explanation (even just a short one) when you 👎 proposals, please?
Contributors need to know why their work is not accepted in order to improve.

[Santhosh-Sellavel]

On @Harshdeepjoshi proposal,

Doing this would solve the issue but will alter the height of the input field slightly.

Not on board with changing the component itself as it might cause unexpected regressions. So gave 👎

Sorry, somehow I missed @Puneet-here's comment!
@liyamahendra's Proposal is the same as Puneet's.

So I like @Puneet-here Proposal here. Looks good to me!
🎀 👀 🎀 C+ reviewed

cc: @Luke9389

[Luke9389]

OK cool, go ahead ands start a PR @Puneet-here!

[Luke9389]

Now that this is assigned I'm going to bump it down to weekly.

[Christinadobrzyn]

Hired in Upwork

• @Puneet-here for the fix
• @Santhosh-Sellavel as the C+ with a $250 bonus for reporting

[Puneet-here]

The PR is ready
cc: @Luke9389 @Santhosh-Sellavel

[Luke9389]

I'll let @Santhosh-Sellavel do the initial review.

[Santhosh-Sellavel]

@Luke9389 PR is already merged

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.79-17 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• add focus indicator for emoji picker search field #9534

If no regressions arise, payment will be issued on 2022-07-15. 🎊

[Christinadobrzyn]

Looks like no regressions.
Paid
@Puneet-here for the fix
@Santhosh-Sellavel as the C+ with a $250 bonus for reporting

closing job and this GH.