archived report banner - word break, border radius and bottom margin

[srikarparsi]

Details

Fixed the border radius and margin from the bottom as reported in the issue. I also changed the word break of the text. I'm not sure what is expected behavior but the "because" was split into two lines originally which didn't look right:

Fixed Issues

$ #13373
PROPOSAL: GH_LINK_ISSUE(COMMENT)

Tests

1. If you have an archived workspace, skip to step 3
2. Create a new workspace
3. Archive the workspace
4. Ensure that on mobile applications, the archived report banner is a little above the bottom of the screen as shown in the screenshots
5. Ensure that the border radius also looks like the screenshots

• Verify that no errors appear in the JS console

Offline tests

QA Steps

Details

Fixed Issues

$ GH_LINK
PROPOSAL: GH_LINK_ISSUE(COMMENT)

Tests

• Verify that no errors appear in the JS console

Offline tests

QA Steps

• 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:
  • iOS / native
  • Android / native
  • iOS / Safari
  • Android / Chrome
  • MacOS / Chrome
  • 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.

Screenshots/Videos

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

[melvin-bot]

@mananjadhav @Luke9389 One of you needs to 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]

[srikarparsi]

Currently, main isn't working on main which was reported here but I ran it on full screen and resized the view to mobile on both safari and chrome for the screenshots.

[srikarparsi]

hey @shawnborton and @Luke9389, I wrote this in the description but I wasn't able to find the expected behavior of the text in the archived room banner. The text "because" was split across two lines and I changed the word break from all to just split the words. I just wanted to ask in case we do want to split by letters as it looks in the pr description.

[shawnborton]

Hmm I have no idea why we'd want to split the letters, that feels weird to me. Do we even need to force a split here? Can we just let the container dictate where the line break happens naturally?

[srikarparsi]

Sorry, I'm not really following. The container can either decide to go to the next line based on the individual characters or the words.

letters:

words:

It was initially set to start a new line based on letters but I changed it to use words so that "because" doesn't get split into two lines. Is this what you were recommending or could I do something else?

[shawnborton]

Yup, we’re on the same page. Let’s break on words and not letters.

…

On Sun, Dec 11, 2022 at 8:05 PM Srikar Parsi ***@***.***> wrote: Sorry, I'm not really following. The container can either decide to go to the next line based on the individual characters or the words. letters: [image: Screen Shot 2022-12-08 at 11 16 28 PM] <https://user-images.githubusercontent.com/48188732/206640746-5f06d598-23d3-4835-ba26-780e4720dc35.png> words: [image: Screen Shot 2022-12-09 at 1 23 02 AM] <https://user-images.githubusercontent.com/48188732/206639583-e0bdc4d9-402b-4e6c-8d5e-7e93b7de6a10.png> It was initially set to start a new line based on letters but I changed it to use words. Is this what you were recommending or is it something else? — Reply to this email directly, view it on GitHub <#13467 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AARWH5VCEKRD7LNWOBOPMMTWM2QABANCNFSM6AAAAAASY4SMEI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[srikarparsi]

We thought about incorporating the changes from this issue into this PR but since the C+ already approved a solution, I think we'll leave it to them so this should be ready to review @mananjadhav @Luke9389

[Luke9389]

Looks great, thanks!

[mananjadhav]

I'll get to this today!

[mananjadhav]

@srikarparsi Can you please update the Tests/QA Steps?

[mananjadhav]

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 tests pass on all platforms & I tested again on:
  • iOS / native
  • Android / native
  • iOS / Safari
  • Android / Chrome
  • MacOS / Chrome
  • 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 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 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.js 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(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 reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

[mananjadhav]

@srikarparsi Are you aware if the Web login issue is resolved? It still isn't working for me.

Meanwhile I posted if you could update the Testing/QA steps.

[mananjadhav]

@srikarparsi I am done with the testing on all platforms. Just left with the Testing steps and we can move this to finish line.

[srikarparsi]

thanks @mananjadhav! I just added the test steps so it should be ready.

[mananjadhav]

Completed the checklist. Thanks @srikarparsi for the update

[Luke9389]

Hmmm, checklist checks are still failing. I'll restart them....

[Luke9389]

Hey, @srikarparsi. Would you re-do your checklist with this one?
https://raw.githubusercontent.com/Expensify/App/main/.github/PULL_REQUEST_TEMPLATE.md

[srikarparsi]

Details

Fixed Issues

$ GH_LINK
PROPOSAL: GH_LINK_ISSUE(COMMENT)

Tests

• Verify that no errors appear in the JS console

Offline tests

QA Steps

• 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:
  • iOS / native
  • Android / native
  • iOS / Safari
  • Android / Chrome
  • MacOS / Chrome
  • 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.

Screenshots/Videos

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

[srikarparsi]

cool it worked, thanks luke! @mananjadhav, could you please redo the reviewer checklist with this

[mananjadhav]

I just updated the comment with the new checklist I generally copy the checklist from the link only. @srikarparsi Can we rerun the GH action?

[Luke9389]

I'll rerun it now. 🤞

[Luke9389]

@shawnborton, any comments before we merge?

[shawnborton]

Looks good to me!

[srikarparsi]

@Luke9389, @shawnborton, is this good to merge?

[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
TTI	783.907 ms → 811.743 ms (+27.836 ms, +3.6%)
runJsBundle	178.677 ms → 191.600 ms (+12.923 ms, +7.2%)
regularAppStart	0.016 ms → 0.016 ms (-0.000 ms, -1.1%)
nativeLaunch	9.621 ms → 9.161 ms (-0.459 ms, -4.8%)

Show details

Name	Duration
TTI	Baseline Mean: 783.907 ms Stdev: 36.919 ms (4.7%) Runs: 737.2404800001532 738.2204760001041 740.7473030001856 741.1159660001285 742.9332630001009 753.0566580002196 753.0920620001853 755.5998669997789 761.3087380002253 763.1058950000443 763.1065509999171 764.109956999775 766.6998850000091 768.2198049998842 768.9271289999597 769.7288119997829 775.5709069999866 777.9743440002203 778.4659529998899 780.2771330000833 801.295274999924 802.4087009998038 809.5796799999662 810.3517289999872 812.3107360000722 814.3100640000775 832.3117829998955 833.6197009999305 848.6173160001636 865.564176000189 871.2370839999057 Current Mean: 811.743 ms Stdev: 40.482 ms (5.0%) Runs: 732.6725940001197 748.7907130001113 750.9976559998468 752.6142370002344 776.6016049999744 780.1753519997001 789.9210390001535 795.5928259999491 796.249007999897 798.4449089998379 801.6629759999923 802.3442830001004 804.4218000001274 804.5034150001593 807.914718999993 808.89940200001 813.2676610001363 816.982598000206 818.0471689999104 818.467505000066 819.9933650000021 821.1985760000534 821.5513100000098 824.2979810000397 828.1346539999358 854.8759710001759 881.4960030000657 882.3223680001684 898.1984450002201 901.6458069998771
runJsBundle	Baseline Mean: 178.677 ms Stdev: 18.335 ms (10.3%) Runs: 148 156 156 158 161 161 163 167 168 168 169 170 171 171 173 176 176 179 181 184 185 186 186 187 190 192 195 205 213 220 224 Current Mean: 191.600 ms Stdev: 20.183 ms (10.5%) Runs: 154 160 164 169 170 176 177 180 181 181 182 182 183 184 186 187 189 195 198 200 201 203 205 208 211 218 219 220 225 240
regularAppStart	Baseline Mean: 0.016 ms Stdev: 0.001 ms (6.2%) Runs: 0.014525999780744314 0.014566999860107899 0.0147299999371171 0.014771000016480684 0.014812000095844269 0.014892000239342451 0.014892000239342451 0.015015000011771917 0.01509599993005395 0.015176999848335981 0.015298999845981598 0.015339999925345182 0.015381000004708767 0.015503000002354383 0.015583999920636415 0.015585000161081553 0.015828000381588936 0.015910000074654818 0.01615400006994605 0.016316999681293964 0.01631700014695525 0.0163569999858737 0.016398000065237284 0.016398000065237284 0.016642999835312366 0.016846000216901302 0.017822000198066235 0.0188400000333786 Current Mean: 0.016 ms Stdev: 0.002 ms (12.0%) Runs: 0.01310200011357665 0.013346000574529171 0.013712000101804733 0.013712000101804733 0.01375299971550703 0.01375299971550703 0.013915999792516232 0.014078999869525433 0.014200999867171049 0.014282000251114368 0.014444999862462282 0.01464799977838993 0.014689000323414803 0.014689000323414803 0.015015000011771917 0.015054999850690365 0.01509599993005395 0.01525900000706315 0.01534100016579032 0.015585000161081553 0.015665999613702297 0.015868999995291233 0.016316999681293964 0.0163569999858737 0.016844999976456165 0.01704900013282895 0.01725299982354045 0.01769999973475933 0.01855500042438507 0.01904299994930625 0.019286999944597483 0.020791999995708466
nativeLaunch	Baseline Mean: 9.621 ms Stdev: 1.324 ms (13.8%) Runs: 8 8 8 8 8 9 9 9 9 9 9 9 9 9 9 9 9 10 10 10 10 10 11 11 11 11 11 13 13 Current Mean: 9.161 ms Stdev: 0.919 ms (10.0%) Runs: 8 8 8 8 8 8 8 8 8 8 9 9 9 9 9 9 9 10 10 10 10 10 10 10 10 10 10 10 10 10 11

[OSBotify]

🚀 Deployed to staging by @Luke9389 in version: 1.2.42-0 🚀

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

[OSBotify]

🚀 Deployed to production by @yuwenmemon in version: 1.2.42-2 🚀

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