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

fix(chat): Fix user tagging in messages #1765

Merged
merged 22 commits into from
Feb 1, 2024
Merged

fix(chat): Fix user tagging in messages #1765

merged 22 commits into from
Feb 1, 2024

Conversation

Flemmli97
Copy link
Collaborator

What this PR does 📖

  • Fixes tagging a user not working in chat anymore:
    HTML tags where escaped after the div indicating tags where added causing it to render the div as plain text.
    This PR moves it after they are escaped but it comes with a caveat:
    The process of tagging a user runs as follows:
    User tags someone in their message using @Name#ShortID -> converted to @DID.
    On receiving side did gets converted to @Name (and previously this was cached but its now not possible anymore).
    TBH though this caveat might just be a case of micro optimization.

@github-actions github-actions bot added Don't merge yet DO NOT MERGE Missing dev review Still needs to be reviewed by a dev labels Jan 25, 2024
@github-actions github-actions bot added the linter failing Cargo Workflow (linter) failed on this PR label Jan 25, 2024
@phillsatellite
Copy link
Contributor

phillsatellite commented Jan 25, 2024

Noticed a few weird things while testing with Sara

So it seems that anyone Im not friends with, the tag will just show as a did is that intended? Sara tagged my old Phill account (capital P) and since Im not friends with that account Im unable to see it

Captura_de_ecra_2024-01-25_as_15 56 35 Screenshot_2024-01-25_at_10 55 44_AM

Something I noticed too that was different from how we have it on dev branch if you type out something like header wrapped in h1 tags and wrap it in backticks is shows like this

Screenshot 2024-01-25 at 11 14 00 AM

compared to dev that looks like a spoiler message

Screenshot 2024-01-25 at 11 17 33 AM

common/src/state/mod.rs Outdated Show resolved Hide resolved
common/src/warp_runner/ui_adapter/mod.rs Outdated Show resolved Hide resolved
common/src/warp_runner/ui_adapter/mod.rs Outdated Show resolved Hide resolved
@phillsatellite phillsatellite added the Changes requested When other dev or QA request a change label Jan 25, 2024
Copy link
Contributor

github-actions bot commented Jan 25, 2024

UI Automated Test Results Summary for MacOS/Windows

491 tests   434 ✅  2h 8m 4s ⏱️
 41 suites   57 💤
  3 files      0 ❌

Results for commit 90623ad.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Jan 25, 2024

UI Automated Tests execution is complete! You can find the test results report here

@Flemmli97
Copy link
Collaborator Author

Noticed a few weird things while testing with Sara

So it seems that anyone Im not friends with, the tag will just show as a did is that intended? Sara tagged my old Phill account (capital P) and since Im not friends with that account Im unable to see it

Captura_de_ecra_2024-01-25_as_15 56 35 Screenshot_2024-01-25_at_10 55 44_AM
Something I noticed too that was different from how we have it on dev branch if you type out something like header wrapped in h1 tags and wrap it in backticks is shows like this

Screenshot 2024-01-25 at 11 14 00 AM compared to dev that looks like a spoiler message Screenshot 2024-01-25 at 11 17 33 AM

it is taking the identity from state. so if state has not resolved the correct name it will take the incorrect one.

as for the 2. thing: prob just needed to update this branch to dev? as i dont see how this pr should affect it

@Flemmli97 Flemmli97 removed the linter failing Cargo Workflow (linter) failed on this PR label Jan 25, 2024
@stavares843 stavares843 removed the Changes requested When other dev or QA request a change label Jan 25, 2024
@github-actions github-actions bot added the missing fixing conflict A conflict exists in current PR and needs resolution label Jan 25, 2024
@Flemmli97 Flemmli97 removed the missing fixing conflict A conflict exists in current PR and needs resolution label Jan 26, 2024
@phillsatellite phillsatellite added the QA Tested QA has tested and approved label Jan 29, 2024
@github-actions github-actions bot added Failed Automated Test This PR is failing Luis's Appium test and needs revised and removed Failed Automated Test This PR is failing Luis's Appium test and needs revised labels Jan 31, 2024
@dariusc93 dariusc93 added Ready to Merge This PR is ready to merge and removed Missing dev review Still needs to be reviewed by a dev Don't merge yet DO NOT MERGE labels Feb 1, 2024
@stavares843 stavares843 added Waiting for CI Waiting for at least one CI job to complete before merging and removed QA Tested QA has tested and approved Ready to Merge This PR is ready to merge labels Feb 1, 2024
@stavares843 stavares843 merged commit b100aa3 into dev Feb 1, 2024
4 of 5 checks passed
@stavares843 stavares843 deleted the user_tag_fix branch February 1, 2024 15:42
@github-actions github-actions bot removed the Waiting for CI Waiting for at least one CI job to complete before merging label Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants