[Hold of payment Aug 17th] Update form styles on Expensify.cash mobile app

[Christinadobrzyn]

Platform - version: E.cash app v1.0.38.-1

Action Performed (reproducible steps):

• Go to Expensify.cash
• View any field on the mobile app (Phone or Email sign-in filed, preferences, change password, search, etc)
• Currently, you'll see a grey field label on an empty field > tap the field to select it
• The field will go white with a blue border > the grey field will disappear
• We want to change this so the grey field label remains while typing
• Here's examples of the current layout and the desired layout:

Current:

Desired:

Specific specs:

• Text size should be 13pt for field label and 15pt for text

• Keeping our existing border style (blue when focused/active, red when error/active) and button style (green for primary actions)

• Field inside the box should be white (both in a selected field and unselected field)

• Field label text should be grey (both in a selected field and unselected field)

• These field changes need to be made to all fields on the mobile app

Expected Result:

• Every selected field without an error should show a grey field label at the top of the field, have a blue border and a green highlighted action button

• Every selected with an error field should show a grey field label at the top of the field, have a red border and a green highlighted action button

Actual Result:

• Currently, every selected field hides the grey field label

Notes/Photos/Videos:

Here's a view of how this should display:

PLEASE NOTE: This should not be applied in the main text box where "Write something" displays, this should keep the following behaviour:

[Christinadobrzyn]

Waiting for Eng review of this issue - https://github.com/Expensify/Expensify/issues/157830 - before uploading to Upwork

[kakajann]

Hello, I would like to work on this. One of my previous work was mostly about styling (Growl Notification ).

[Christinadobrzyn]

I'm going to be ooo May 31 - June 7th so assigning this to Kadie to post in Upwork once we have a thumbs up on the internal GH - https://github.com/Expensify/Expensify/issues/157830

[MelvinBot]

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

[kadiealexander]

@Beamanator I'm yet to post this on Upwork but we already have proposals so I've added the label.

[Beamanator]

@kakajann Thanks for your interest in this issue! Would you please provide write up a proposal of the work you'd plan to do? Please also ask any questions you may have about the desired "Expected Result".

[kakajann]

Proposal

React Native doesn't support :focus selector on styling. So, we have to create custom TextInput with react natives TextInput if we want to change TextInputs border color when user focus on it.

Example:

import React, { Component } from 'react';
import { TextInput, StyleSheet } from 'react-native';

const styles = StyleSheet.create({
  textInput: {
    fontSize: 15,
    borderColor: '#dddddd'
  },
  textInput: {
    borderColor: '#00ff00'
  },
})

class ExpensifyTextInput extends Component {
  constructor() {
    super()

    this.state = {
      focused: false
    }
  }

  render() {
    const {focused} = this.state
    return (
      <TextInput
        style={[styles.textInput, focused && styles.textInputOnFocus]}
        onFocus={() => this.setState({focused: true})}
        onBlur={() => this.setState({focused: false})}
      />
    )
  }
}

export default ExpensifyTextInput

I will also implement specific specs that mentioned in the issue.

Actual Result:
Currently, every selected field hides the grey field label

And this is because outline hasn't disabled in here

[Beamanator]

@kakajann From your proposal I'm not sure you've clearly read / understood what changes are needed.

So, we have to create custom TextInput with react natives TextInput if we want to change TextInputs border color when user focus on it.

The spec says "Keeping our existing border style (blue when focused/active, red when error/active)", so you shouldn't need to build anything extra for border colors. The main change in this issue is about the field placeholder / label text, can you explain how you think you would implement those requested changes?

[kadiealexander]

Upwork link: https://www.upwork.com/jobs/~01a91a3d33f29673bc

[mnrafg]

Hello Expensify Team,

I hope you are all doing fine.

About Me

I'm Nasir Rahimi a freelance full-stack developer with almost a decade of practical experience and following is my Upwork profile link.
https://www.upwork.com/freelancers/~0189e2e63b0663c9b4

Proposal

I suggest two options to achieve the expected and resulting TextInput.

First Option: Customize and Use Third-Party Component

It is an option to use such a component from a third-party stable library, which has almost the same functionality but needs some style changes to meet the expected UI requirements an example I can mention here now is the extended TextInput component from the react-native-paper library.

You can see the examples by visiting the following link: https://callstack.github.io/react-native-paper/text-input.html

Second Option: A Custom Extension of TextInput Component

The second option could be the straightforward extension of the built-in ReactNative TextInput, which will result in our own new TextInput component.

Note: the following code is a basic draft idea about the implementation for the complete solution I'll cheat from the above-mentioned react-native-paper library and carefully study how they extended the native TextInput component for that you can read the TextInputOutlined component code by visiting the following link.

https://github.com/callstack/react-native-paper/blob/40253f5b07a87812315c16a61a6bc19edb077638/src/components/TextInput/TextInputOutlined.tsx

class TextInput extends React.Component {
  // ...
  render() {
    const {label, ...inputProps} = this.props;

    return (
      // Wrapper View
      <View>
        {
        // An extended and customized View as Outline
        // which will be visible as input outline or border.
        }
        <Outline />
        {
        // An extended and customized Text
        // which will be visible as input label.
        }
        <Label value={label} />
        {
        // An extended and customized Text
        // which will be visible as input label.
        }
        <TextInput {...inputProps} />
      </View>
    )
  }
}

I hope you have got the path I'll follow for solving this issue, but I want to mention that I'll need a few days for this to implement because I'm currently having other projects as well.

Regards,
Nasir Rahimi

[kakajann]

@Beamanator , No inputs currently change its border color on focus. (I'll attach a video below) That's why I suggested to create custom TextInput. Also, creating a custom TextInput component will make it easy to implement shrinking labels.

I can provide a working sample if you me to do. It's easier than explaining :)

And here is current inputs (that doesn't change its border color)

Screen.Recording.2021-06-05.at.7.10.02.PM.mov

[Beamanator]

@mnrafg Thanks for your proposal and your multiple ideas! I believe your second solution (A Custom Extension of TextInput Component) could be the best solution for what we're looking for, but I am planning to follow up with our team internally to make sure nobody has any concerns about this plan :D I'll get back to you soon! (most likely tomorrow)

[Beamanator]

@kakajann Thanks for the video, I do see what you mean now. I believe react-native-web is adding data-focusvisible-polyfill="true" to our inputs, which is giving our text fields the blue borders on practically all fields in web, different colored borders in mobile web (for Android at least), and no borders in mobile (except for the chat message text field). Now I'm pretty sure the borders I'm seeing are browser-specific, not necessarily related to something react-native-web is doing.

Your solution could work great for border styling, but I didn't see a clear path forward for how you planned to implement the label-shrinking in the new custom TextInput component. Feel free to add a proposal with details on that if you'd like!

[MelvinBot]

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

[Beamanator]

@kakajann I added the Design label to assign someone from the Design team to look at the PR - looks like @sylveawong is the lucky one :) Also @kakajann could you remove [WIP] from your PR title if it's ready for review, and assign pullerbear?

[shawnborton]

I don't mind grabbing this one as @kakajann already tagged me in the PR.

[kakajann]

I have no idea how to request a review from pullerbear. any help would be appreciated)

[roryabraham]

@Beamanator @kakajann Readonly permissions (as all our Open-Source contributors have) aren't enough to request a reviewer in GitHub. See https://docs.github.com/en/organizations/managing-access-to-your-organizations-repositories/repository-permission-levels-for-an-organization for more details.

@kakajann We have PullerBear set up as a code owner, so as soon as you create a PR his review will be automatically requested, and then an Expensify engineer will be automatically assigned to review your code. So you shouldn't need to do anything 😁

[Beamanator]

@roryabraham for reference, here is @kakajann 's PR: #3414

It looks like when he originally created the PR, it did assign pullerbear & later Shawn re-requested pullerbear to review, which is how I'm the current reviewer 👍 I was on vacation yesterday so I'll get to this today!

Readonly permissions (as all our Open-Source contributors have) aren't enough to request a reviewer in GitHub. See https://docs.github.com/en/organizations/managing-access-to-your-organizations-repositories/repository-permission-levels-for-an-organization for more details.

That's great to know! Thanks for sharing :)

[Christinadobrzyn]

Just checking on this PR, it looks like we're still working through some checks. Let me know if that's incorrect!

[kakajann]

I had to work on this for more than 2 months because of delayed code reviews and unexpected conflicts. Also, I got to add new features.

Therefore, I am requesting to increase the budget for this work.

[parasharrajat]

I support @kakajann here. PR looks heavy.

[Beamanator]

@kakajann thanks for bringing this up, we're discussing internally and I definitely agree you should get a pay bump for this big task. Can you please give us an estimated # of hours you've worked on this related PR?

[kakajann]

I have no idea to be honest. Yesterday only, spent 6 hours 36 minutes (wakatime's result) to refactor

[MelvinBot]

@shawnborton, @Beamanator, @kakajann, @Christinadobrzyn it looks like no one is assigned to work on this job.
Please double the price or add a comment stating why the job isn't being doubled.

[Beamanator]

Job price in Upwork will be doubled today 👍

[Christinadobrzyn]

It looks like the original Upwork job expired so I created a new one with the $500 price and hired @kakajann

Internal job post - https://www.upwork.com/ab/applicants/1422830164452577280/job-details
External job post - https://www.upwork.com/jobs/~010f4ad7e03243ba0c

[parasharrajat]

Finally, this is landed. App on a whole new level.

[anthony-hull]

Congrats great work :)

[MitchExpensify]

This is an unintended consequence of this change right?

Slack thread

[Luke9389]

I am making a PR to shorten the copy inside the text input. There is a separate regression caused by this PR that's already been identified here.

[Christinadobrzyn]

Reopening this because of the regression, I think that's the right next step? @Luke9389 do you know if this should be reopened while the regression is resolved?

[Luke9389]

I'm not sure whether we need to keep this open or not. I think it's functionally pretty much the same. The regression I mentioned above is already being fixed, so the contributor in this issue (@kakajann) doesn't have any actionable steps to take.

[Christinadobrzyn]

Paid @kakajann in Upwork! Upwork job closed.