Skip to content

Search - Pasting long text removes left padding & cursor focus is missing #58981

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

thelullabyy
Copy link
Contributor

@thelullabyy thelullabyy commented Mar 24, 2025

Explanation of Change

Fixed Issues

$#58214
PROPOSAL:#58214 (comment)

Tests

  1. Open Ap
  2. Tap the Search icon at the top
  3. Paste a short text into the search input
  4. Observe that the text has proper left padding, and the focus is on the cursor
  5. Remove the text
  6. Paste a long text into the search input
  7. Verify the focusing cursor remains visible at the end of the pasted text and the left padding is consistent for both short and long pasted text.
  • Verify that no errors appear in the JS console

Offline tests

QA Steps

  1. Open Ap
  2. Tap the Search icon at the top
  3. Paste a short text into the search input
  4. Observe that the text has proper left padding, and the focus is on the cursor
  5. Remove the text
  6. Paste a long text into the search input
  7. Verify the focusing cursor remains visible at the end of the pasted text and the left padding is consistent for both short and long pasted text.
  • Verify that no errors appear in the JS console

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: mWeb Chrome
    • iOS: Native
    • iOS: mWeb 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
      • If any non-english text was added/modified, I used JaimeGPT to get English > Spanish translation. I then posted it in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
    • 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 grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
    • 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.ts 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 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(theme.componentBG))
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • 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 the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • 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 added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native
android.mov
Android: mWeb Chrome
android_chorme.mov
iOS: Native
Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-03-24.at.16.58.06.mp4
iOS: mWeb Safari
ios_safari.mp4
MacOS: Chrome / Safari
chorme.mov
MacOS: Desktop
desktop.mov

@thelullabyy thelullabyy marked this pull request as ready for review March 24, 2025 09:59
@thelullabyy thelullabyy requested a review from a team as a code owner March 24, 2025 09:59
@melvin-bot melvin-bot bot requested review from mananjadhav and removed request for a team March 24, 2025 09:59
Copy link

melvin-bot bot commented Mar 24, 2025

@mananjadhav Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

@mananjadhav
Copy link
Collaborator

@thelullabyy I am trying to load the PR. But the video doesn't load for me. Can you check once more at your side? I tried reloading a few times and the other PRs work just fine for me.

image

@thelullabyy
Copy link
Contributor Author

@mananjadhav Hi, all videos or just for MacOs and desktop that you can't load. I still watch normally. If you still can't load this, I will update and notify you.

Screen.Recording.2025-03-25.at.19.08.25.mov

@mananjadhav
Copy link
Collaborator

Not sure if something changed but I am able to see all the videos now! Weird. I'll now start with the review and testing. @thelullabyy have you done the testing on a few other fields? The proposal mentioned that it's an issue with all the fields. Hence it would be best if we can test a few scenarios to check if it does fix and doesn't cause any regressions. A recording only for the Web should also suffice.

@thelullabyy
Copy link
Contributor Author

@mananjadhav I have tested on some other fields here. No problems found.

Screen.Recording.2025-03-26.at.09.59.50.mov
Screen.Recording.2025-03-26.at.09.mp4

@mananjadhav
Copy link
Collaborator

mananjadhav commented Mar 27, 2025

Reviewer Checklist

  • I have verified the author checklist is complete (all boxes are checked off).
  • I verified the correct issue is linked in the ### Fixed Issues section above
  • I verified testing steps are clear and they cover the changes made in this PR
    • I verified the steps for local testing are in the Tests section
    • I verified the steps for Staging and/or Production testing are in the QA steps section
    • I verified the steps cover any possible 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 checked that screenshots or videos are included for tests on all platforms
  • I included screenshots or videos for tests on all platforms
  • I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
  • I verified tests pass on all platforms & I tested again on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
    • MacOS: Desktop
  • If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
  • I verified proper code patterns were followed (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 grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
    • 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 verified that this PR follows the guidelines as stated in the Review Guidelines
  • I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
  • 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.ts or at the top of the file that uses the constant) are defined as such
  • 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(theme.componentBG)
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • 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 the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • 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.
  • For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
  • I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native
android-input-padding.mov
Android: mWeb Chrome mweb-chrome-padding
iOS: Native
ios-input-padding.mov
iOS: mWeb Safari
mweb-safari-input-padding.mov
MacOS: Chrome / Safari
web-input-padding.mov
MacOS: Desktop
desktop-input-padding.mov

@mananjadhav
Copy link
Collaborator

@thelullabyy This doesn't work for me on Android chrome. The left padding is still not there when I paste long text. It works fine for other platforms.

image

@mananjadhav
Copy link
Collaborator

@thelullabyy quick bump on the PR

@thelullabyy
Copy link
Contributor Author

thelullabyy commented Mar 31, 2025

@mananjadhav Sorry for my delay time. I updated PR and tested work well on others platform. This is video for Android chrome, pls help to check. Thank you!

Screen.Recording.2025-03-31.at.15.54.21.mov
Screen.Recording.2025-03-31.at.15.51.37.mov

@mananjadhav
Copy link
Collaborator

Thanks for the update. I'll review this in a while.

@melvin-bot melvin-bot bot requested a review from marcochavezf April 1, 2025 18:37
@mananjadhav
Copy link
Collaborator

Checklist here #58981 (comment)

Copy link
Contributor

@marcochavezf marcochavezf left a comment

Choose a reason for hiding this comment

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

Thanks guys!

@marcochavezf marcochavezf merged commit 9ccdde5 into Expensify:main Apr 2, 2025
18 checks passed
@OSBotify
Copy link
Contributor

OSBotify commented Apr 2, 2025

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

Copy link
Contributor

github-actions bot commented Apr 2, 2025

Performance Comparison Report 📊⚠️ Some tests did not pass successfully, so some results are omitted from final report: Linking

Significant Changes To Duration

Name Duration
App start time contentAppeared_To_screenTTI 1075.592 ms → 1233.115 ms (+157.523 ms, +14.6%) 🔴
App start time TTI 1769.309 ms → 1920.141 ms (+150.832 ms, +8.5%) 🔴
Show details
Name Duration
App start time contentAppeared_To_screenTTI Baseline
Mean: 1075.592 ms
Stdev: 262.721 ms (24.4%)
Runs: 531.4145269999281 560.7752840002067 574.4141299999319 591.840307999868 619.0389919998124 642.5331749999896 655.8626689999364 666.3844229998067 666.9419840001501 669.5352719998918 719.1504560001194 719.5461699999869 743.5740660000592 750.255495000165 757.5884929997846 762.2036120002158 803.9561700001359 1095.2216770001687 1096.5474709998816 1108.7895720000379 1139.1683990000747 1149.4764689998701 1154.0968089997768 1157.7722450001165 1161.6605960000306 1172.9775209999643 1173.4311009999365 1181.6785510000773 1184.4289190000854 1185.1350699998438 1194.865825000219 1199.019975000061 1223.9219360002317 1228.6368499998935 1229.560792000033 1232.860317000188 1239.1086380002089 1239.2708850000054 1240.42534600012 1241.449614000041 1244.2791120000184 1251.570813999977 1258.9988620001823 1268.8650460001081 1272.2980390000157 1274.397867999971 1284.349018999841 1286.2319519999437 1288.3430809997953 1288.8267879998311 1298.1966809998266 1301.3789980001748 1308.3794789998792 1310.2681370000355 1310.4481739997864 1316.7398600000888 1319.9299880000763 1326.5668089999817 1328.8234950001352 1332.1088769999333

Current
Mean: 1233.115 ms
Stdev: 79.867 ms (6.5%)
Runs: 955.4356570001692 1060.8650509999134 1069.8336749998853 1118.9347560000606 1132.6295810001902 1134.6751890000887 1146.8295519999228 1146.9468970000744 1154.5336270001717 1155.8387009999715 1165.0455570002086 1181.4107169997878 1197.3665319997817 1197.7701880000532 1211.740954999812 1212.78588199988 1219.8109349999577 1221.841905000154 1222.858514000196 1227.5795559999533 1242.0066109998152 1242.3180490001105 1242.5646669999696 1242.5984939998016 1242.883479999844 1245.0725899999961 1254.2719359998591 1257.3153789998032 1260.5628669997677 1261.121431000065 1263.7657639998943 1266.8967820000835 1268.440144999884 1270.4816700001247 1274.065847999882 1286.2853049999103 1288.2522829999216 1290.2352479998954 1292.5618010000326 1293.1077769999392 1294.0815039998852 1299.751147000119 1299.81057399977 1301.2400299999863 1319.5378970000893 1321.302585999947 1321.8572160000913 1349.8321369998157 1353.6610429999419 1375.1483370000497
App start time TTI Baseline
Mean: 1769.309 ms
Stdev: 277.174 ms (15.7%)
Runs: 1246.0389919998124 1263.5331749999896 1266.3844229998067 1268.5352719998918 1281.7752840002067 1291.840307999868 1319.9419840001501 1344.255495000165 1361.8626689999364 1362.546169999987 1367.414526999928 1374.5740660000592 1381.1504560001194 1414.414129999932 1471.5884929997846 1493.2036120002158 1504.956170000136 1735.7895720000379 1765.7722450001165 1776.1683990000747 1779.6605960000306 1791.5474709998816 1799.0968089997768 1817.4311009999365 1826.1350699998438 1827.4289190000854 1853.019975000061 1853.9219360002317 1856.570813999977 1859.2216770001687 1866.449614000041 1866.6785510000773 1880.6368499998935 1886.2791120000184 1905.560792000033 1930.42534600012 1947.9775209999643 1955.860317000188 1964.397867999971 1986.4764689998701 1987.7398600000888 1991.1966809998266 1991.8650460001081 1994.2980390000157 1997.8234950001352 2000.4481739997864 2003.3430809997953 2005.2319519999437 2005.2708850000054 2006.3794789998792 2011.1088769999333 2013.3789980001748 2015.349018999841 2022.865825000219 2026.2681370000355 2032.8267879998311 2051.5668089999817 2060.9299880000763 2066.108638000209 2127.9988620001823

Current
Mean: 1920.141 ms
Stdev: 121.457 ms (6.3%)
Runs: 1468.4155629999004 1675.4356570001692 1686.8336749998853 1743.8650509999134 1757.9347560000606 1761.0455570002086 1779.6751890000887 1810.4107169997878 1825.9468970000744 1848.6295810001902 1848.78588199988 1852.8295519999228 1853.8109349999577 1860.3153789998032 1871.3665319997817 1871.841905000154 1882.7701880000532 1888.3180490001105 1891.5628669997677 1898.8387009999715 1899.5646669999696 1899.8967820000835 1905.0066109998152 1908.7657639998943 1924.740954999812 1928.440144999884 1939.4816700001247 1949.0725899999961 1952.2719359998591 1957.121431000065 1957.5336270001717 1962.5795559999533 1989.883479999844 1993.5984939998016 1994.2522829999216 2009.2853049999103 2015.8321369998157 2017.5618010000326 2019.065847999882 2023.302585999947 2028.2352479998954 2029.0815039998852 2029.2400299999863 2039.8572160000913 2041.751147000119 2043.6610429999419 2050.1077769999392 2054.5378970000893 2060.81057399977 2096.858514000196 2127.1483370000497

Meaningless Changes To Duration

Show entries
Name Duration
App start time nativeLaunchEnd_To_appCreationStart 81.259 ms → 75.625 ms (-5.634 ms, -6.9%)
App start time nativeLaunch 24.237 ms → 24.153 ms (-0.085 ms, ±0.0%)
App start time appCreation 63.810 ms → 66.333 ms (+2.523 ms, +4.0%)
App start time appCreationEnd_To_contentAppeared 519.034 ms → 522.898 ms (+3.864 ms, +0.7%)
App start time runJsBundle 321.017 ms → 320.517 ms (-0.500 ms, ±0.0%)
App start time regularAppStart 0.021 ms → 0.020 ms (-0.001 ms, -2.5%)
App start time (CPU) 149.141 % → 148.682 % (-0.459 %, ±0.0%)
App start time (FPS) 60.000 FPS → 60.000 FPS
App start time (RAM) 385.496 MB → 385.749 MB (+0.253 MB, ±0.0%)
App start time (CPU/JS) 0.000 % → 0.000 %
App start time (CPU/UI) 25.293 % → 24.691 % (-0.602 %, -2.4%)
Open search router TTI Open Search Router TTI 1340.438 ms → 1330.530 ms (-9.908 ms, -0.7%)
Open search router TTI Load Search Options 175.634 ms → 175.409 ms (-0.226 ms, ±0.0%)
Open search router TTI (CPU) 140.729 % → 141.668 % (+0.939 %, +0.7%)
Open search router TTI (FPS) 60.000 FPS → 60.000 FPS
Open search router TTI (RAM) 412.671 MB → 414.385 MB (+1.714 MB, ±0.0%)
Open search router TTI (CPU/JS) 0.000 % → 0.000 %
Open search router TTI (CPU/UI) 22.277 % → 22.692 % (+0.414 %, +1.9%)
Report typing Composer typing rerender count 1.000 renders → 1.000 renders
Report typing Message sent 472.495 ms → 472.752 ms (+0.257 ms, ±0.0%)
Report typing (CPU) 97.547 % → 96.709 % (-0.838 %, -0.9%)
Report typing (FPS) 60.000 FPS → 60.000 FPS
Report typing (RAM) 468.023 MB → 466.887 MB (-1.136 MB, ±0.0%)
Report typing (CPU/JS) 0.000 % → 0.000 %
Report typing (CPU/UI) 20.954 % → 20.872 % (-0.082 %, ±0.0%)
Chat opening Chat TTI 981.352 ms → 980.880 ms (-0.471 ms, ±0.0%)
Chat opening (CPU) 154.604 % → 151.629 % (-2.975 %, -1.9%)
Chat opening (FPS) 60.000 FPS → 60.000 FPS
Chat opening (RAM) 407.299 MB → 405.383 MB (-1.915 MB, ±0.0%)
Chat opening (CPU/JS) 0.000 % → 0.000 %
Chat opening (CPU/UI) 30.552 % → 29.650 % (-0.902 %, -3.0%)

undefined

@github-actions github-actions bot added the DeployBlockerCash This issue or pull request should block deployment label Apr 2, 2025
Copy link
Contributor

github-actions bot commented Apr 2, 2025

@Expensify/mobile-deployers 📣 Please look into this performance regression as it's a deploy blocker.

Copy link
Contributor

github-actions bot commented Apr 3, 2025

🚀 Deployed to staging by https://github.com/marcochavezf in version: 9.1.23-1 🚀

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

Copy link
Contributor

github-actions bot commented Apr 7, 2025

🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.1.23-7 🚀

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DeployBlockerCash This issue or pull request should block deployment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants