Text & Dropdown Input Styles

[Luke9389]

Details

Fixed Issues

$ #12445
$ #13741

Tests

• Verify that no errors appear in the JS console

What to do:

Verify that the inputs and dropdowns look like the following:

Focused	Unfocused	Error	Dropdown

Additionally, be sure to check for off-looking background colors.

❌	✅

Where to do it

1. Every settings page
2. FAB ->
   • New Chat Search Input
   • New Group Search Input
   • New Room Search Input
   • Send Money Search Input
   • Request Money Search Input
   • Split Bill Search Input
3. Search Bar at the top of the LHN

QA Steps

• Verify that no errors appear in the JS console

Same as above tests

PR Author Checklist

• I linked the correct issue in the ### Fixed Issues section above
• I wrote clear testing steps that cover the changes made in this PR
  • I added steps for local testing in the Tests section
  • I added steps for the expected offline behavior in the Offline steps section
  • I added steps for Staging and/or Production testing in the QA steps section
  • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
  • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
• I included screenshots or videos for tests on all platforms
• I ran the tests on all platforms & verified they passed on:
  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop
• I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
• I followed proper code patterns (see Reviewing the code)
  • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
  • I verified that comments were added to code that is not self explanatory
  • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
  • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
  • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
  • I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
  • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
  • I verified the JSDocs style guidelines (in STYLE.md) were followed
• If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
• I followed the guidelines as stated in the Review Guidelines
• I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
• I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
• I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
• I verified that if a function's arguments changed that all usages have also been updated correctly
• If a new component is created I verified that:
  • A similar component doesn't exist in the codebase
  • All props are defined accurately and each prop has a /** comment above it */
  • The file is named correctly
  • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
  • The only data being stored in the state is data necessary for rendering and nothing else
  • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
  • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
  • All JSX used for rendering exists in the render method
  • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
• If any new file was added I verified that:
  • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• If a new CSS style is added I verified that:
  • A similar style doesn't already exist
  • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
• I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

PR Reviewer Checklist

The reviewer will copy/paste it into a new comment and complete it after the author checklist is completed

- [x] I linked the correct issue in the `### Fixed Issues` section above - [x] I wrote clear testing steps that cover the changes made in this PR - [x] I added steps for local testing in the `Tests` section - [x] I added steps for the expected offline behavior in the `Offline steps` section - [x] I added steps for Staging and/or Production testing in the `QA steps` section - [x] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct) - [x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline) - [x] I tested this PR with a [High Traffic account](https://github.com/Expensify/App/blob/main/contributingGuides/CONTRIBUTING.md#high-traffic-accounts) against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability). - [x] I included screenshots or videos for tests on [all platforms](https://github.com/Expensify/App/blob/main/contributingGuides/CONTRIBUTING.md#make-sure-you-can-test-on-all-platforms) - [x] I ran the tests on **all platforms** & verified they passed on: - [x] Android / native - [x] Android / Chrome - [x] iOS / native - [x] iOS / Safari - [x] MacOS / Chrome / Safari - [x] MacOS / Desktop - [x] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed) - [x] I followed proper code patterns (see [Reviewing the code](https://github.com/Expensify/App/blob/main/contributingGuides/PR_REVIEW_GUIDELINES.md#reviewing-the-code)) - [x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. `toggleReport` and not `onIconClick`) - [x] I verified that comments were added to code that is not self explanatory - [x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing. - [x] I verified any copy / text shown in the product is localized by adding it to `src/languages/*` files and using the [translation method](https://github.com/Expensify/App/blob/4bd99402cebdf4d7394e0d1f260879ea238197eb/src/components/withLocalize.js#L60) - [x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the [localization methods](https://github.com/Expensify/App/blob/4bd99402cebdf4d7394e0d1f260879ea238197eb/src/components/withLocalize.js#L60-L68) - [x] I verified any copy / text that was added to the app is correct English and approved by marketing by adding the `Waiting for Copy` label for a copy review on the original GH to get the correct copy. - [x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README. - [x] I verified the JSDocs style guidelines (in [`STYLE.md`](https://github.com/Expensify/App/blob/main/contributingGuides/STYLE.md#jsdocs)) were followed - [x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers - [x] I followed the guidelines as stated in the [Review Guidelines](https://github.com/Expensify/App/blob/main/contributingGuides/PR_REVIEW_GUIDELINES.md) - [x] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like `Avatar`, I verified the components using `Avatar` are working as expected) - [x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests) - [x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such - [x] I verified that if a function's arguments changed that all usages have also been updated correctly - [x] If a new component is created I verified that: - [x] A similar component doesn't exist in the codebase - [x] All props are defined accurately and each prop has a `/** comment above it */` - [x] The file is named correctly - [x] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone - [x] The only data being stored in the state is data necessary for rendering and nothing else - [x] For Class Components, any internal methods passed to components event handlers are bound to `this` properly so there are no scoping issues (i.e. for `onClick={this.submit}` the method `this.submit` should be bound to `this` in the constructor) - [x] Any internal methods bound to `this` are necessary to be bound (i.e. avoid `this.submit = this.submit.bind(this);` if `this.submit` is never passed to a component event handler like `onClick`) - [x] All JSX used for rendering exists in the render method - [x] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions - [x] If any new file was added I verified that: - [x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory - [x] If a new CSS style is added I verified that: - [x] A similar style doesn't already exist - [x] The style can't be created with an existing [StyleUtils](https://github.com/Expensify/App/blob/main/src/styles/StyleUtils.js) function (i.e. `StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG`) - [x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like `Avatar` is modified, I verified that `Avatar` is working as expected in all cases) - [x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected. - [x] If a new page is added, I verified it's using the `ScrollView` component to make it scrollable when more elements are added to the page. - [x] I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

[Luke9389]

This PR is held on the color swapping PR (so we can fully test it in its completed form).

[shawnborton]

Are we planning to wait until the color changes are finished before merging this? Otherwise I would say this could be merged first since all it does is change shapes and styles and not colors.

[Luke9389]

@shawnborton, when testing this I came across this difference that gave me pause. Let me know what you think.

	Old	New
Zoomed In
In Context

Does the text on the new one look a little scrunched to the left to you?

Here it is with no background color, but then it gets hard to distinguish that from the active/clickable ones.

[shawnborton]

Personally I think it looks great!

…

On Mon, Dec 5, 2022 at 7:31 PM Luke Donahue ***@***.***> wrote: @shawnborton <https://github.com/shawnborton>, when testing this I came across this difference that gave me pause. Let me know what you think. Old New Zoomed In [image: Screen Shot 2022-12-05 at 7 27 16 PM] <https://user-images.githubusercontent.com/49007721/205805881-551193ea-a33d-47bc-bfee-917f18d42d02.png> [image: Screen Shot 2022-12-05 at 7 27 07 PM] <https://user-images.githubusercontent.com/49007721/205805903-30ed6433-4d36-4480-a8bf-aa4a29fdaab6.png> In Context [image: Screen Shot 2022-12-05 at 7 29 22 PM] <https://user-images.githubusercontent.com/49007721/205806107-31c7cbab-5ef3-43cd-9a3d-762052e3f4c9.png> [image: Screen Shot 2022-12-05 at 7 29 51 PM] <https://user-images.githubusercontent.com/49007721/205806125-8bf1ef20-9322-4c0d-bf28-14b9539ffc32.png> — Reply to this email directly, view it on GitHub <#12447 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AARWH5XSIMVELA3CJ5S2E53WL2XPZANCNFSM6AAAAAARWP2LXA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[Luke9389]

Awesome! That's definitely the path of least effort for me! 😄

[Luke9389]

OK, we've only got one use of that inputHeightSmall variable, and it's on the small locale picker at the bottom of the login page. I just made that change to inputHeightSmall: 28 and retested it real quick to verify that it works.

[shawnborton]

Dope, thanks for taking care of that! I think it's probably safe to merge this now, but let me know if we need to redo the screenshots.

[OSBotify]

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

[github-actions]

Performance Comparison Report 📊

Significant Changes To Duration

There are no entries

Meaningless Changes To Duration

Show entries

Name	Duration
App start TTI	714.764 ms → 738.019 ms (+23.255 ms, +3.3%)
App start runJsBundle	197.548 ms → 203.774 ms (+6.226 ms, +3.2%)
Open Search Page TTI	611.629 ms → 613.424 ms (+1.795 ms, ±0.0%)
App start nativeLaunch	20.586 ms → 21.677 ms (+1.091 ms, +5.3%)
App start regularAppStart	0.015 ms → 0.016 ms (+0.001 ms, +6.1%)

Show details

Name	Duration
App start TTI	Baseline Mean: 714.764 ms Stdev: 25.638 ms (3.6%) Runs: 667.4225929994136 675.6157469991595 676.5836340002716 676.7284629996866 683.7960110008717 685.3549400009215 687.4587999992073 691.2576039992273 693.5569650009274 704.1337269991636 706.4519619997591 708.6731400005519 711.6444109994918 712.5175020005554 713.947771999985 714.5387169998139 716.9346619993448 718.5757790002972 721.9989219997078 725.9784869998693 728.7339820005 731.8295559994876 733.8623539991677 736.4113069996238 738.9759490005672 739.3879519999027 740.6750419996679 743.1707680001855 748.3107779994607 751.7141270004213 771.442446000874 Current Mean: 738.019 ms Stdev: 24.207 ms (3.3%) Runs: 691.0738290008157 699.1681500002742 704.8350930009037 707.2816610001028 713.2925160005689 714.0752559993416 717.0793660003692 717.3940839990973 720.8865879997611 722.7221579998732 725.8934010006487 726.9532709997147 734.0906390007585 735.2935440000147 737.2303449995816 741.7755720000714 742.949109999463 743.7830960005522 744.4589709993452 745.7133670002222 746.3159449994564 746.4379500001669 747.7948659993708 750.9145679995418 759.8396620005369 760.5794750005007 761.8072800002992 766.1092649996281 772.7080830000341 789.3691780008376 790.7767980005592
App start runJsBundle	Baseline Mean: 197.548 ms Stdev: 15.210 ms (7.7%) Runs: 169 175 178 181 181 183 187 187 189 189 191 192 193 193 193 194 197 198 199 200 201 202 204 208 208 210 216 224 225 225 232 Current Mean: 203.774 ms Stdev: 14.050 ms (6.9%) Runs: 186 186 187 187 191 192 193 194 194 195 195 196 196 200 201 203 203 204 204 205 205 207 207 210 212 214 218 222 236 236 238
Open Search Page TTI	Baseline Mean: 611.629 ms Stdev: 16.710 ms (2.7%) Runs: 576.6076250001788 582.6032720003277 585.7684330008924 587.3272299990058 588.7258299998939 592.4836430009454 594.831380000338 596.2482509985566 600.4270829986781 601.035237999633 609.5530609991401 611.3932289984077 613.0060220006853 613.5430509988219 614.012085000053 614.2956949993968 614.8945310004056 615.9864909984171 616.3621420003474 616.6972250007093 617.4114989992231 617.6104330010712 619.3842370007187 622.9386400002986 624.1818439997733 625.0968829989433 625.1265060007572 626.0281990002841 631.1546630002558 632.7360430005938 633.3291829992086 651.3188480008394 Current Mean: 613.424 ms Stdev: 19.826 ms (3.2%) Runs: 582.6308599989861 585.5790210012347 589.9302989989519 591.0213620010763 592.019328000024 592.0505379997194 593.5438640005887 593.9477949999273 594.18481499888 597.0768640004098 597.4550369996578 601.6492929998785 603.6388349998742 607.9316410012543 609.3209639992565 611.2399899996817 613.6429040003568 615.8557130005211 616.7887779995799 617.355671999976 617.4023439995944 620.9442139994353 623.4443769995123 625.4123950004578 625.7714840006083 628.1304519996047 629.0923670008779 630.1357420012355 632.7804359998554 646.1330980006605 646.515788000077 649.8535560015589 660.4976000003517
App start nativeLaunch	Baseline Mean: 20.586 ms Stdev: 1.451 ms (7.0%) Runs: 19 19 19 19 19 19 19 19 20 20 20 20 20 20 21 21 21 21 21 21 21 21 21 21 22 22 22 24 25 Current Mean: 21.677 ms Stdev: 2.146 ms (9.9%) Runs: 19 19 20 20 20 20 20 20 20 20 20 20 21 21 21 21 21 21 22 22 22 22 22 23 23 23 24 25 26 27 27
App start regularAppStart	Baseline Mean: 0.015 ms Stdev: 0.001 ms (7.5%) Runs: 0.013182999566197395 0.013225000351667404 0.013671999797224998 0.013671999797224998 0.013753000646829605 0.013753000646829605 0.013833999633789062 0.0138349998742342 0.0138349998742342 0.014038000255823135 0.014241999015212059 0.01448499970138073 0.01448499970138073 0.014526000246405602 0.014526000246405602 0.014608001336455345 0.014689000323414803 0.014730000868439674 0.014812000095844269 0.01505500078201294 0.01505500078201294 0.015095999464392662 0.015544001013040543 0.015584999695420265 0.01574699953198433 0.015949999913573265 0.016520000994205475 0.016804998740553856 0.01700899936258793 0.017619000747799873 Current Mean: 0.016 ms Stdev: 0.001 ms (5.5%) Runs: 0.013631001114845276 0.01464799977838993 0.014892000705003738 0.014973999932408333 0.015014998614788055 0.015015000477433205 0.015095999464392662 0.01521800085902214 0.01521800085902214 0.015257999300956726 0.015298999845981598 0.01534000039100647 0.015421001240611076 0.01550300046801567 0.015625 0.015665000304579735 0.015706000849604607 0.015707001090049744 0.015707001090049744 0.01574699953198433 0.015786999836564064 0.0157880000770092 0.0157880000770092 0.016154000535607338 0.016316000372171402 0.016398001462221146 0.016560999676585197 0.016642000526189804 0.01676500029861927 0.01794400066137314 0.018025999888777733

[OSBotify]

🚀 Deployed to staging by https://github.com/Luke9389 in version: 1.2.60-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

[Prince-Mendiratta]

I believe there's been a regression with this PR. Please check out this comment for Issue, RCA and solution!

cc @Santhosh-Sellavel @shawnborton @Luke9389 @parasharrajat

[Luke9389]

Hi @Prince-Mendiratta.

Thanks for calling that out. I'm away today but I'll fix that tomorrow.

Much appreciated!

[kavimuru]

@luke we see the change in login field as well. But not given in this PR. Should we create a GH for it.

[Luke9389]

@kavimuru I intended this PR to cover the login as well. Sorry if that was unclear. It touches pretty much every single input.

[OSBotify]

🚀 Deployed to production by https://github.com/luacmartins in version: 1.2.60-1 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

[luacmartins]

FYI this PR caused this issue. It's an edge case of a single text input that has a different style.

[parasharrajat]

Ah, I see we missed it. Everyone was pushing this PR and we got stuck with a different issue so I missed it during the review. Thanks for taking care of that.

[aldo-expensify]

Following bugzero checklist: I think this change caused this: #14632

Thanks @situchan for pointing that out.

[parasharrajat]

Issues with RoomInput are expected as it was missed during testing because of being behind beta.

There was one more issue found before, check last comment before this.

[aldo-expensify]

Makes sense :)

[luacmartins]

FYI this PR introduced this bug, since the hint style wasn't updated accordingly.

[parasharrajat]

Ok, I think that is a kind of improvement as I didn't think that hint needs to be left aligned as well on this PR.