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] Editing a comment containing both RTL and LTR characters breaks the (edited) text - Reported by @adeel0202 #9709

Closed
mvtglobally opened this issue Jul 5, 2022 · 27 comments
Assignees
Labels
Engineering External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Reviewing Has a PR in review Weekly KSv2

Comments

@mvtglobally
Copy link

mvtglobally commented Jul 5, 2022

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Go to any chat
  2. Type something containing RTL characters
  3. Send the comment
  4. Edit the comment and add LTR characters
  5. Press Save changes

Expected Result:

The (edited) text doesn't break

Actual Result:

The (edited) text breaks

Workaround:

unknown

Platform:

Where is this issue occurring?

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Version Number: 1.1.79-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2022-06-13.at.9.17.49.PM.mov

Upwork URL: https://www.upwork.com/jobs/~01c6ba7a40ecceeb34
Issue reported by: @adeel0202
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1655138110274269

View all open jobs on GitHub

@mvtglobally mvtglobally added AutoAssignerTriage Auto assign issues for triage to an available triage team member Daily KSv2 labels Jul 5, 2022
@melvin-bot
Copy link

melvin-bot bot commented Jul 5, 2022

Triggered auto assignment to @trjExpensify (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

@melvin-bot melvin-bot bot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Jul 5, 2022
@melvin-bot
Copy link

melvin-bot bot commented Jul 5, 2022

Triggered auto assignment to @sketchydroide (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@trjExpensify trjExpensify added the Improvement Item broken or needs improvement. label Jul 5, 2022
@trjExpensify
Copy link
Contributor

Passing on to engineering triage. Will remain assigned for CM if it can go external.

@sketchydroide sketchydroide added the External Added to denote the issue can be worked on by a contributor label Jul 6, 2022
@melvin-bot
Copy link

melvin-bot bot commented Jul 6, 2022

Current assignee @trjExpensify is eligible for the External assigner, not assigning anyone new.

@sketchydroide
Copy link
Contributor

I think this can be external.

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Jul 8, 2022
@melvin-bot
Copy link

melvin-bot bot commented Jul 8, 2022

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 8, 2022
@melvin-bot
Copy link

melvin-bot bot commented Jul 8, 2022

Current assignee @sketchydroide is eligible for the Exported assigner, not assigning anyone new.

@melvin-bot melvin-bot bot changed the title Editing a comment containing both RTL and LTR characters breaks the (edited) text - Reported by @adeel0202 [$250] Editing a comment containing both RTL and LTR characters breaks the (edited) text - Reported by @adeel0202 Jul 8, 2022
@trjExpensify
Copy link
Contributor

Oops, someone missed this. On upwork here: https://www.upwork.com/jobs/~01c6ba7a40ecceeb34

@Karim-30
Copy link
Contributor

Karim-30 commented Jul 8, 2022

Proposal

wrap the comment and edit Texts inside a <View>. So change these lines inside ReportActionItemFragment.js to these :

<View style={{flexDirection:'row',alignItems:'center'}}>

    <Text
        selectable={!canUseTouchScreen() || !props.isSmallScreenWidth}
        style={EmojiUtils.containsOnlyEmojis(props.fragment.text) ? styles.onlyEmojisText : undefined}
    >
        {Str.htmlDecode(props.fragment.text)}
    </Text>

    {props.fragment.isEdited && (
        <Text
            fontSize={variables.fontSizeSmall}
            color={themeColors.textSupporting}
        >
            {` ${props.translate('reportActionCompose.edited')}`}
        </Text>
    )}

</View>

Result :
RTL

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jul 8, 2022
@sketchydroide
Copy link
Contributor

@Karim-30 can you show me an example with just edited RTL text?
And add a before and after example.

thanks

@Karim-30
Copy link
Contributor

Karim-30 commented Jul 8, 2022

@sketchydroide I updated my Proposal, here is how it looks like

before :

before.mov

after :

after.mov

@rushatgabhane
Copy link
Member

@Karim-30 I don't think we should vertically center (edited)
image

@jeet-dhandha
Copy link
Contributor

Just a question does Expensify provide with payments for the solutions provided by freelance developers, as they can just fix the code from the provided proposal without giving out the payments.

@rushatgabhane
Copy link
Member

rushatgabhane commented Jul 10, 2022

@jeet-dhandha good question. If an issue is labeled External and your proposal is accepted, you'll be paid.

I'd strongly recommend that you read https://github.com/Expensify/App/blob/main/contributingGuides/CONTRIBUTING.md#propose-a-solution-for-the-job

@Karim-30
Copy link
Contributor

I don't think we should vertically center (edited)

@rushatgabhane You are right, using alignItems: 'baseline' instead of alignItems: 'center' fixed it.

Result :

fix alignment

@rushatgabhane
Copy link
Member

🎀 👀 🎀 C+ reviewed
@sketchydroide I like @Karim-30's proposal.

@sketchydroide
Copy link
Contributor

Yeah, I'm happy with it as well

@melvin-bot melvin-bot bot removed Help Wanted Apply this label when an issue is open to proposals by contributors Daily KSv2 labels Jul 11, 2022
@melvin-bot
Copy link

melvin-bot bot commented Jul 11, 2022

📣 @Karim-30 You have been assigned to this job by @sketchydroide!
Please apply to this job in Upwork 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 📖

@melvin-bot melvin-bot bot added the Weekly KSv2 label Jul 11, 2022
@trjExpensify
Copy link
Contributor

@Karim-30 can you apply to the job on upwork when you get a chance?

@Karim-30
Copy link
Contributor

Applied

@Karim-30
Copy link
Contributor

PR is ready

@melvin-bot melvin-bot bot added the Overdue label Jul 25, 2022
@trjExpensify trjExpensify added the Reviewing Has a PR in review label Jul 25, 2022
@melvin-bot melvin-bot bot removed the Overdue label Jul 25, 2022
@trjExpensify
Copy link
Contributor

PR is in review. @sketchydroide looks like it's awaiting your approval and we can merge this?

@rushatgabhane
Copy link
Member

@trjExpensify just FYI, we had a regression #10123
(so half pay for C+)

@rushatgabhane
Copy link
Member

rushatgabhane commented Aug 9, 2022

@trjExpensify we can close this out (commenting because the bot won't remind as the reviewing label has been applied)

@trjExpensify
Copy link
Contributor

Ah, why didn't the labels get added? 🤔 Processing payments now.

@trjExpensify
Copy link
Contributor

Cool, done!

@rushatgabhane
Copy link
Member

rushatgabhane commented Aug 10, 2022

why didn't the labels get added

I think it's because we didn't remove the reviewing label after merging the PR.

If I'm not wrong, the reviewing label disables Melvin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

6 participants