Skip to content
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

[$250] Tweak UI for deleted message and expense #53617

Open
dubielzyk-expensify opened this issue Dec 5, 2024 · 44 comments
Open

[$250] Tweak UI for deleted message and expense #53617

dubielzyk-expensify opened this issue Dec 5, 2024 · 44 comments
Assignees
Labels
Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.

Comments

@dubielzyk-expensify
Copy link
Contributor

dubielzyk-expensify commented Dec 5, 2024

Problem

Today when you delete a message or expense with threads, the chat message gets replaced with [Deleted message] or [Deleted expense]. This feels a bit unpolished:

CleanShot 2024-12-05 at 13 59 52@2x

Solution

Let's spruce up the UI and reuse our attachement when showing a deleted message or expense:

CleanShot 2024-12-09 at 10 24 35@2x

For the receipt deleted we should use the icon already in our repo (App/assets/images/receipt-slash.svg). For the comment deleted, we should use this new icon:
chatbubble-slash.svg.zip

cc @Expensify/design @JmillsExpensify @saracouto

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021864619157987293637
  • Upwork Job ID: 1864619157987293637
  • Last Price Increase: 2024-12-19
  • Automatic offers:
    • shubham1206agra | Reviewer | 105467437
    • Krishna2323 | Contributor | 105467438
Issue OwnerCurrent Issue Owner: @shubham1206agra
Issue OwnerCurrent Issue Owner: @shubham1206agra
@dubielzyk-expensify dubielzyk-expensify added Improvement Item broken or needs improvement. Engineering labels Dec 5, 2024
Copy link

melvin-bot bot commented Dec 5, 2024

Auto-assigning issues to engineers is no longer supported. If you think this issue should receive engineering attention, please raise it in #whatsnext.

@Krishna2323
Copy link
Contributor

Krishna2323 commented Dec 5, 2024

Edited by proposal-police: This proposal was edited at 2024-12-05 05:53:14 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Tweak UI for deleted message and expense

What is the root cause of that problem?

Improvement

What changes do you think we should make in order to solve the problem?

  • First we should create a custom rendered in HTMLEngineProviderComponentList, we can call it 'deleted-action': DeletedActionRenderer,.
    const HTMLEngineProviderComponentList: CustomTagRendererRecord = {
    // Standard HTML tag renderers
    a: AnchorRenderer,
    code: CodeRenderer,
    img: ImageRenderer,
    video: VideoRenderer,
    // Custom tag renderers
    edited: EditedRenderer,
    pre: PreRenderer,
    /* eslint-disable @typescript-eslint/naming-convention */
    'mention-user': MentionUserRenderer,
    'mention-report': MentionReportRenderer,
    'mention-here': MentionHereRenderer,
    emoji: EmojiRenderer,
    'next-step-email': NextStepEmailRenderer,
    /* eslint-enable @typescript-eslint/naming-convention */
    };
  • Create a new component DeletedActionRenderer.
import React from 'react';
import {View} from 'react-native';
import type {CustomRendererProps, TPhrasing, TText} from 'react-native-render-html';
import Icon from '@components/Icon';
import * as Expensicons from '@components/Icon/Expensicons';
import Text from '@components/Text';
import useLocalize from '@hooks/useLocalize';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import CONST from '@src/CONST';
import type {TranslationPaths} from '@src/languages/types';

function DeletedActionRenderer({tnode}: CustomRendererProps<TText | TPhrasing>) {
    const styles = useThemeStyles();
    const theme = useTheme();
    const {translate} = useLocalize();

    const actionTypes = Object.values(CONST.REPORT.ACTION_TYPE);
    const action = tnode.attributes.action;
    let translation = action;

    if (actionTypes.some((e) => e === action)) {
        translation = translate(`parentReportAction.${action}` as TranslationPaths);
    }

    Object.values(CONST.REPORT.ACTION_TYPE);

    const getIcon = () => {
        // This needs to be updated with new icons
        switch (action) {
            case CONST.REPORT.ACTION_TYPE.DELETED_MESSAGE:
                return {icon: Expensicons.ReceiptSlash, width: 18, height: 18};
            case CONST.REPORT.ACTION_TYPE.DELETED_EXPENSE:
                return {icon: Expensicons.ReceiptSlash, width: 18, height: 18};
            case CONST.REPORT.ACTION_TYPE.DELETED_REPORT:
                return {icon: Expensicons.ReceiptSlash, width: 18, height: 18};
            case CONST.REPORT.ACTION_TYPE.DELETED_TASK:
                return {icon: Expensicons.ReceiptSlash, width: 18, height: 18};
            case CONST.REPORT.ACTION_TYPE.HIDDEN_MESSAGE:
                return {icon: Expensicons.ReceiptSlash, width: 18, height: 18};
            case CONST.REPORT.ACTION_TYPE.REVERSED_TRANSACTION:
                return {icon: Expensicons.ReceiptSlash, width: 18, height: 18};
            default:
                return {icon: Expensicons.ReceiptSlash, width: 18, height: 18};
        }
    };

    const icon = getIcon();
    return (
        <View style={[styles.p4, styles.mt1, styles.border, {borderColor: theme.border}, styles.flexRow, styles.justifyContentCenter, styles.alignItemsCenter, styles.gap2]}>
            <Icon
                width={icon.height}
                height={icon.width}
                fill={theme.icon}
                src={icon.icon}
            />
            <Text style={(styles.textLabelSupporting, styles.textStrong)}>{translation}</Text>
        </View>
    );
}

DeletedActionRenderer.displayName = 'DeletedActionRenderer';

export default DeletedActionRenderer;

  • Add the icons in @components/Icon/Expensicons.
  • From everywhere we are using these values of parentReportAction, we should check if it is being used to render <RenderHTML html={and replace it with<RenderHTML html={<deleted-action action="${CONST.REPORT.ACTION_TYPE.DELETED_MESSAGE}"></deleted-action>} />`.

    App/src/languages/en.ts

    Lines 4775 to 4782 in e25e3fa

    parentReportAction: {
    deletedReport: '[Deleted report]',
    deletedMessage: '[Deleted message]',
    deletedExpense: '[Deleted expense]',
    reversedTransaction: '[Reversed transaction]',
    deletedTask: '[Deleted task]',
    hiddenMessage: '[Hidden message]',
    },

    if (isDeletedParentAction || isReversedTransaction) {
    let message: TranslationPaths;
    if (isReversedTransaction) {
    message = 'parentReportAction.reversedTransaction';
    } else {
    message = 'parentReportAction.deletedExpense';
    }
    return <RenderHTML html={`<comment>${translate(message)}</comment>`} />;
    }

    if (isDeletedParentAction) {
    return <RenderHTML html={`<comment>${translate('parentReportAction.deletedTask')}</comment>`} />;
    }

    } else if (action.actionName === CONST.REPORT.ACTIONS.TYPE.REPORT_PREVIEW) {
    children = ReportUtils.isClosedExpenseReportWithNoExpenses(iouReport) ? (
    <RenderHTML html={`<comment>${translate('parentReportAction.deletedReport')}</comment>`} />

    if (isReversedTransaction) {
    message = 'parentReportAction.reversedTransaction';
    } else {
    message = 'parentReportAction.deletedExpense';
    }

    <RenderHTML html={`<comment>${translate('parentReportAction.deletedTask')}</comment>`} />

    if ((!isOffline && isThreadParentMessage && isPendingDelete) || fragment?.isDeletedParentAction) {
    return <RenderHTML html={`<comment>${translate('parentReportAction.deletedMessage')}</comment>`} />;
    }
    if (isThreadParentMessage && moderationDecision === CONST.MODERATION.MODERATOR_DECISION_PENDING_REMOVE) {
    return <RenderHTML html={`<comment>${translate('parentReportAction.hiddenMessage')}</comment>`} />;
  • Add the customHTMLElementModel for deleted-action in BaseHTMLEngineProvider it will look similar to alert-text.
    function BaseHTMLEngineProvider({textSelectable = false, children, enableExperimentalBRCollapsing = false}: BaseHTMLEngineProviderProps) {
    const styles = useThemeStyles();
    // Declare nonstandard tags and their content model here
    /* eslint-disable @typescript-eslint/naming-convention */
    const customHTMLElementModels = useMemo(
    () => ({
    edited: HTMLElementModel.fromCustomModel({
    tagName: 'edited',
    contentModel: HTMLContentModel.textual,
    }),
    'alert-text': HTMLElementModel.fromCustomModel({
    tagName: 'alert-text',
    mixedUAStyles: {...styles.formError, ...styles.mb0},
    contentModel: HTMLContentModel.block,
    }),
  • Create actions types in CONST file.
        ACTION_TYPE: {
            DELETED_MESSAGE: 'deletedMessage',
            DELETED_REPORT: 'deletedReport',
            DELETED_EXPENSE: 'deletedExpense',
            REVERSED_TRANSACTION: 'reversedTransaction',
            DELETED_TASK: 'deletedTask',
            HIDDEN_MESSAGE: 'hiddenMessage',
        },

TEST BRANCH

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?


What alternative solutions did you explore? (Optional)

@melvin-bot melvin-bot bot added the Daily KSv2 label Dec 5, 2024
@Krishna2323
Copy link
Contributor

Krishna2323 commented Dec 5, 2024

@dubielzyk-expensify, these messages are also shown in similar way, do we want to update these too?

App/src/languages/en.ts

Lines 4775 to 4782 in e25e3fa

parentReportAction: {
deletedReport: '[Deleted report]',
deletedMessage: '[Deleted message]',
deletedExpense: '[Deleted expense]',
reversedTransaction: '[Reversed transaction]',
deletedTask: '[Deleted task]',
hiddenMessage: '[Hidden message]',
},

NOTE: I have hidden my proposal for now, will bring back when we are ready with the designs and this issue is made external.

@JmillsExpensify JmillsExpensify added NewFeature Something to build that is a new item. External Added to denote the issue can be worked on by a contributor labels Dec 5, 2024
Copy link

melvin-bot bot commented Dec 5, 2024

Job added to Upwork: https://www.upwork.com/jobs/~021864619157987293637

@melvin-bot melvin-bot bot changed the title Tweak UI for deleted message and expense [$250] Tweak UI for deleted message and expense Dec 5, 2024
Copy link

melvin-bot bot commented Dec 5, 2024

Triggered auto assignment to @MitchExpensify (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

@melvin-bot melvin-bot bot added Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2 and removed Daily KSv2 labels Dec 5, 2024
@JmillsExpensify JmillsExpensify added Daily KSv2 and removed Engineering Weekly KSv2 Improvement Item broken or needs improvement. Help Wanted Apply this label when an issue is open to proposals by contributors labels Dec 5, 2024
Copy link

melvin-bot bot commented Dec 5, 2024

⚠️ It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time ⚠️

Copy link

melvin-bot bot commented Dec 5, 2024

Triggered auto assignment to Contributor-plus team member for initial proposal review - @shubham1206agra (External)

Copy link

melvin-bot bot commented Dec 5, 2024

Triggered auto assignment to Design team member for new feature review - @dannymcclain (NewFeature)

@JmillsExpensify
Copy link

Added the labels to get this one going.

@trjExpensify
Copy link
Contributor

@dubielzyk-expensify, these messages are also shown in similar way, do we want to update these too?

I'd say yes, such that the design is consistent across them all.

@allgandalf
Copy link
Contributor

@Krishna2323 , there is pending action on the PR:

Please complete pending work before posting on new issues :))

@Krishna2323
Copy link
Contributor

@allgandalf, I don’t think I need to wait to close all my PRs before posting proposals, especially when I’m actively working on them. Yesterday, I provided as many updates as I could, and the last question was posted 12 hours ago. Do I need to resolve those instantly every time?

As you can see, I have no open assigned issues at the moment. I always try to complete issues before posting proposals, but it’s not entirely within my control since most of the time, we go through discussions.

@melvin-bot melvin-bot bot added the Overdue label Dec 12, 2024
Copy link

melvin-bot bot commented Dec 16, 2024

@MitchExpensify, @shubham1206agra, @dubielzyk-expensify Eep! 4 days overdue now. Issues have feelings too...

@shubham1206agra
Copy link
Contributor

Waiting for updated UI.

@melvin-bot melvin-bot bot removed the Overdue label Dec 16, 2024
Copy link

melvin-bot bot commented Dec 19, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Dec 19, 2024
@shubham1206agra
Copy link
Contributor

Waiting for updated UI.

@melvin-bot melvin-bot bot removed the Overdue label Dec 20, 2024
@dannymcclain
Copy link
Contributor

Waiting for updated UI.

What updates are we waiting on here?

Copy link

melvin-bot bot commented Dec 23, 2024

@MitchExpensify, @shubham1206agra, @dubielzyk-expensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@melvin-bot melvin-bot bot added the Overdue label Dec 23, 2024
@MitchExpensify
Copy link
Contributor

Friendly bump @shubham1206agra:

What updates are we waiting on here?

Copy link

melvin-bot bot commented Dec 25, 2024

@MitchExpensify, @shubham1206agra, @dubielzyk-expensify Eep! 4 days overdue now. Issues have feelings too...

@shubham1206agra
Copy link
Contributor

Lets go with @Krishna2323's proposal.

🎀👀🎀 C+ Reviewed

@melvin-bot melvin-bot bot removed the Overdue label Dec 26, 2024
Copy link

melvin-bot bot commented Dec 26, 2024

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

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 26, 2024
Copy link

melvin-bot bot commented Dec 26, 2024

📣 @shubham1206agra 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

Copy link

melvin-bot bot commented Dec 26, 2024

📣 @Krishna2323 🎉 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 📖

Copy link

melvin-bot bot commented Dec 30, 2024

@rlinoz, @MitchExpensify, @shubham1206agra, @Krishna2323, @dubielzyk-expensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added the Overdue label Dec 30, 2024
@MitchExpensify
Copy link
Contributor

@Krishna2323 What is the latest on your PR?

Copy link

melvin-bot bot commented Jan 1, 2025

@rlinoz, @MitchExpensify, @shubham1206agra, @Krishna2323, @dubielzyk-expensify Eep! 4 days overdue now. Issues have feelings too...

Copy link

melvin-bot bot commented Jan 3, 2025

@rlinoz, @MitchExpensify, @shubham1206agra, @Krishna2323, @dubielzyk-expensify Still overdue 6 days?! Let's take care of this!

@rlinoz
Copy link
Contributor

rlinoz commented Jan 3, 2025

Hey @Krishna2323 when can we expect a PR?

@melvin-bot melvin-bot bot removed the Overdue label Jan 3, 2025
@Krishna2323
Copy link
Contributor

Krishna2323 commented Jan 3, 2025

The draft is ready locally, I'll push it tomorrow. I on holidays.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.
Projects
Status: Bugs and Follow Up Issues
Development

No branches or pull requests

10 participants