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

feat: hyperlinks for chat #3252

Merged
merged 65 commits into from
Feb 3, 2025

Conversation

fcolarich
Copy link
Contributor

@fcolarich fcolarich commented Jan 29, 2025

What does this PR change?

Implements #3095
This implements 2 reusable components, a ValidatedInputText, that makes sure all text that is input in the associated TMP_InputText component is properly validated (included when doing backspaces or pasting text) and a reusable HyperlinkHandlerElement that can be put next to any TMP_Text component and will allow to display hyperlinks and react to them correctly.
I also extracted all Input Box functionality into its own element (which in the near future will be further broken down when extracting the emojis logic).
And did a bunch of file movements to break some circular assembly refs, introducing a chat command bus, to send commands through it and reduce the number of dependencies on the chat commands themselves. There are a lot of bad uses of chat commands referencing stuff they shouldnt through dubious paths. We should not do it xD

How to test the changes?

  • Check that all chat interactions work as expected, including commands.

  • While testing all this, try removing characters with backspace or delete keys, to test that links break (or not) when they should.

  • Check that only valid scenes are detected as links (including the weird ass parcels on the right side with weird coords), to detect parcel addresses they must not be preceded or followed by any other character, except a blank space. So -150,150 is valid but a150,150 is not.

  • Check that web addresses are correctly detected as such, accepted formats are:
    http://www.something.cc
    https://www.something.cc
    www.something.cc
    https://something.cc
    something.cc
    it also supports more complex subdomains like www.something.cc/somethingelse

  • Check that world addresses are correctly detected, their format is something.dcl.eth, again, it must not be preceded or followed by any other character (except white spaces).

  • Check that only allowed rich text tags work, for now, only bold and italics work ( and ) any other tag will be converted to a similar character that is inert for our purposes.

  • Check that multiple links of different or same type can coexist in the same message without issues and that they open correctly the expected window when clicked.

  • Check that after sending the messages the message in the chat is shown in color and that when you hover over the link, it gets underlined correctly (only when you hover over it) and when unhovering it should return to its original blue color. When you click on it, depending on the type of link, a popup will show to make sure you wanna open that link or go to that scene/world. Also the cursor should change to a hand thingy when hovering over the link.
    Make sure all types of links work as expected and that no weird behaviour is experienced when hovering (like text getting scrambled or showing rich text tags).

There might be some issues when pasting text that include links, specially when using Ctrl+V. But that will be handled in another PR.

@fcolarich fcolarich self-assigned this Jan 29, 2025
@fcolarich fcolarich requested review from a team as code owners January 29, 2025 23:59
@fcolarich fcolarich requested review from QThund and Ludmilafantaniella and removed request for AnsisMalins, mikhail-dcl and DafGreco January 30, 2025 01:21
@fcolarich fcolarich linked an issue Jan 30, 2025 that may be closed by this pull request
Copy link
Contributor

@QThund QThund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome job! I left some comments, nothing too important, so green light!

@Ludmilafantaniella
Copy link

Ludmilafantaniella commented Jan 31, 2025

Tested on Windows ✅
I ran tests on Windows, and everything is working as expected. Mac testing is still pending due to build failures.

Test performed:

Image

Image

Evidence:

Untitled.Video.-.Made.With.Clipchamp.mp4

@Ludmilafantaniella Ludmilafantaniella mentioned this pull request Jan 31, 2025
4 tasks
Copy link

@Ludmilafantaniella Ludmilafantaniella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on Mac ✅
Mac testing is now complete, and everything is working as expected. Approving the PR.

Final tests performed:

#3252
#3259
Evidence: See the previous Windows comment for evidence and additional test execution details.

✅ Approved by QA

@Ludmilafantaniella Ludmilafantaniella mentioned this pull request Jan 31, 2025
@fcolarich fcolarich merged commit ce29578 into feat/chat/chat_improvements_main Feb 3, 2025
9 checks passed
@fcolarich fcolarich deleted the feat/chat/hyperlinks branch February 3, 2025 11:58
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.

Hyperlinks
3 participants