[$250] Tags - In expense report, full tag name is not shown in tag list page
If you havenβt already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: V9. 0.70-0 Reproducible in staging?: Y Reproducible in production?: Y If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y Issue reported by: Applause Internal Team
Action Performed:
- Go to https://staging.new.expensify.com/home
- Go to workspace settings-- more features
- Enable tags
- Tap tags - add tag
- Paste 1 : ghhhg8gugigiigiggiigiggiigguguigtiyiy8gigigiigy9giigigigig : huigigjggiigkbbjhj : π₯°ππ
- Tap save
- Go to workspace chat
- Create 2 expenses with merchant entered and tag selected
- Note in tag list, full tag name is not shown
Expected Result:
In expense report, full tag name must be shown in tag list page.
Actual Result:
In expense report, full tag name is not shown in tag list page.
Workaround:
Unknown
Platforms:
- [x] Android: Standalone
- [x] Android: HybridApp
- [x] Android: mWeb Chrome
- [ ] iOS: Standalone
- [ ] iOS: HybridApp
- [ ] iOS: mWeb Safari
- [ ] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
https://github.com/user-attachments/assets/64207707-cdf4-4536-83e9-3ec7bd3de096
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~021864790410260297484
- Upwork Job ID: 1864790410260297484
- Last Price Increase: 2024-12-05
Issue Owner
Current Issue Owner: @rushatgabhane
Triggered auto assignment to @MitchExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.
Edited by proposal-police: This proposal was edited at 2024-12-03 14:22:47 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
Tags - In expense report, full tag name is not shown in tag list page
What is the root cause of that problem?
We're using numberOfLine = 1 in
https://github.com/Expensify/App/blob/d9e53c7b8ff8fe17d3536508c83824a246f218b1/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx#L465
What changes do you think we should make in order to solve the problem?
We should remove numberOfLine = 1, then update the style if needed.
We can check other tag places to fix this isuse.
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
It's just the small change depends on design team, so I think we don't need to add tests
Proposal
Please re-state the problem that we are trying to solve in this issue.
handling edge cases where those values may be undefined or null. The logic in the code needs to ensure that if the merchant's name is not available, the description should fall back as a default, or vice versa. Additionally, there may be other situations where the data needs to be split or formatted for display correctly in the MoneyRequestPreviewContent component.
What is the root cause of that problem?
The root cause appears to be a lack of consistent conditional rendering or proper fallback handling in situations where the merchant name or description may be undefined or null. The initial rendering logic does not always check whether these values are available, leading to potential display issues or missing information in certain cases.
The Goal:
- To handle null or undefined values for the merchant name and description gracefully.
- To ensure the content displays correctly based on the available data (e.g., fallbacks or conditional rendering).
App/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx
if (!shouldShowMerchant) {
merchantOrDescription = description || ''; // Adjusted the fallback to '' if description is not provided
}
(getSettledMessage method): Update for clearer readability or better localization handling.
(splitShare calculation): Ensure calculations or checks are optimally returning values where needed for performance or correctness.
similarly. is missing the What changes do you think we should make in order to solve the problem? line, which makes the proposal invalid. Consequently, the response should be:
[User] Your proposal will be dismissed because you did not follow the proposal template.
Please re-state the problem that we are trying to solve in this issue.
Issue with conditionally rendering merchant's name/description in MoneyRequestPreview and MoneyRequestPreviewContent components. Need proper fallback handling when data is missing.
What is the root cause of that problem?
Inconsistent conditional rendering and missing fallback for merchant's name/description, leading to incomplete or incorrect display.
What changes do you think we should make in order to solve the problem?
- Improve conditional rendering to check and display fallback if data is missing.
- Use checks like
merchantName || description || 'Default Text'. - Refactor
MoneyRequestPreviewContentto handle cases when both are missing.
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
- Both name and description present.
- Name missing, description present.
- Name present, description missing.
- Both missing, show default.
- Edge cases (null, undefined, empty string).
What alternative solutions did you explore? (Optional)
Explore using a default rendering component to centralize fallback logic, but improving current component checks seems simpler.
Edited by proposal-police: This proposal was edited at 2024-12-03 15:37:56 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
Tags - In expense report, full tag name is not shown in tag list page
What is the root cause of that problem?
We are not applying the word-break break-all style here in RadioListItem
https://github.com/Expensify/App/blob/64829131fe962681626b4b016140521d05f7fe52/src/components/SelectionList/RadioListItem.tsx#L54-L61
so when the word is too long it will not break it to the next line in tag picker page
and similarly in money request preview we are limiting the number of lines
https://github.com/Expensify/App/blob/d9e53c7b8ff8fe17d3536508c83824a246f218b1/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx#L465
What changes do you think we should make in order to solve the problem?
We need to apply breakAll style styles.breakAll in RadioListItem title style in all cases or if we want to be specific we can create a param and apply the style when the breakAll param is true and pass it wherever needed like in this case for tagpicker
breakAll ? styles.breakAll : null,
I think the only place we should display the full tag is in the above case (in the tag list page) but if we want to apply the same for MoneyRequestPreviewContent
https://github.com/Expensify/App/blob/d9e53c7b8ff8fe17d3536508c83824a246f218b1/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx#L465
we can remove the numberOfLines and also the remove the styles.pre style here ( optionally we can replace it with preWrap if we want)
https://github.com/Expensify/App/blob/64829131fe962681626b4b016140521d05f7fe52/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx#L466
And also if needed do the same for confirmation page by pass 0 as numberOfLinesTitle and applying the appropriate word-break (styles.breakAll) style here or we can set a higher non-zero numberOfLinesTitle value > 2 which could also do the trick. If needed we can also apply the same behaviour of showing full tag in TagSettingPage and money request view(as the default numberofLines of the menu in money request view is set to 1) too, but I really doubt the necesssity
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
What alternative solutions did you explore? (Optional)
It's expected. We don't want to show the full tag name if it's too long. Context: https://github.com/Expensify/App/issues/52261
It's expected. We don't want to show the full tag name if it's too long. Context: #52261
For money request preview I agree with you it is expected but for tag list page users should be able to see the full tag name π
Solution:
Problem:
Long tag names are being fully displayed, making the UI look cluttered. We need to truncate tag names that are too long to fit within a designated space.
Root Cause:
The tag names are being rendered without any length restrictions or truncation applied, causing the content to overflow or appear cluttered.
Proposed Changes:
- Apply truncation to tag names exceeding a certain length (e.g., 20 characters).
- Use ellipsis (
...) for overflow text, such astagName.length > 20 ? tagName.substring(0, 20) + '...' : tagName.
Test Scenarios:
- Test with short and long tag names.
- Test with different container sizes to ensure truncation works across different screen resolutions.
- Test with edge cases like empty or very long tag names (e.g., 100+ characters).
Alternative Solutions:
Consider using a tooltip to display the full tag name on hover or implementing a "See More" feature, but truncating with ellipsis is the simplest approach.
Triggered auto assignment to @dannymcclain (Design), see these Stack Overflow questions for more details.
~~@dannymcclain what do you think would be the best solution here assuming we do want to show the full tag name?~~ Sorry I think we're all set here with the expected behavior being two lines
Two lines seems to be the expected behavior here based on Categories
That makes me wonder why we jumped to line 2 in this example:
Is that the root problem?
Sorry I think we're all set here with the expected behavior being two lines Two lines seems to be the expected behavior here based on Categories
Agree with two lines. I'll also add that this seems like a pretty unrealistic edge case so let's just make this work like categories and be done with it!
Triggered auto assignment to @dannymcclain (
Design), see these Stack Overflow questions for more details.
job invite solution looks ok?
Triggered auto assignment to @dannymcclain (
Design), see these Stack Overflow questions for more details.
its required some code line tweak as well CSS
That makes me wonder why we jumped to line 2 in this example:
Is that the root problem?
Yes that's the problem @MitchExpensify In the current issue case the word (ghhhg8gugigiigiggiigiggiigguguigtiyiy8gigigiigy9giigigigig) is too long (long number of characters without a space) and we haven't set the appropriate style to make the word break to the next line when the word overflows from the current line. So in this case the long word will not be on the first line because it overflows from the line hence it will break to new line on the first space after 1 : and we need to fix that. π
Job added to Upwork: https://www.upwork.com/jobs/~021864790410260297484
Triggered auto assignment to Contributor-plus team member for initial proposal review - @rushatgabhane (External)
@rushatgabhane, @MitchExpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!
Ok so what proposal can we build off here @rushatgabhane ?
@FitseTLT's proposal LGTM πππ
https://github.com/Expensify/App/issues/53467#issuecomment-2514895928
Triggered auto assignment to @blimpich, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
π£ @FitseTLT π An offer has been automatically sent to your Upwork account for the Contributor role π Thanks for contributing to the Expensify app!
Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review π§βπ» Keep in mind: Code of Conduct | Contributing π
Reviewing label has been removed, please complete the "BugZero Checklist".
The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.78-6 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:
- https://github.com/Expensify/App/pull/53940
If no regressions arise, payment will be issued on 2025-01-02. :confetti_ball:
For reference, here are some details about the assignees on this issue:
- @rushatgabhane requires payment through NewDot Manual Requests
- @FitseTLT requires payment automatic offer (Contributor)
@rushatgabhane @MitchExpensify @rushatgabhane The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]
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 Issuessection 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
Testssection - [ ] I verified the steps for Staging and/or Production testing are in the
QA stepssection - [ ] 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 verified the steps for local testing are in the
- [ ] 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:
- [ ] 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.
toggleReportand notonIconClick). - [ ] 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
- [ ] 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.
- [ ] 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 usingAvatarhave 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
thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor) - [ ] Any internal methods bound to
thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick) - [ ] 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
Avataris modified, I verified thatAvataris 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
Designlabel and/or tagged@Expensify/designso the design team can review the changes.
- [ ] If a new page is added, I verified it's using the
ScrollViewcomponent 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
mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps. - [ ] 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: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop
Reminder set to pay, please complete the BZ steps in the meantime @rushatgabhane π
Payment summary:
- $250 @rushatgabhane requires payment through NewDot Manual Requests
- $250 @FitseTLT requires payment automatic offer (Contributor)
@blimpich, @rushatgabhane, @MitchExpensify, @FitseTLT Whoops! This issue is 2 days overdue. Let's get this updated quick!