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

Allow subclasses to customise mention text #1000

Closed
wants to merge 2 commits into from

Conversation

cjmconie
Copy link
Contributor

Submit a pull request

CLA

  • I have signed the Stream CLA (required).
  • The code changes follow best practices
  • Code changes are tested (add some information if not applicable)

Description of the pull request

This PR adds the method mentionText(forUser:) to ChatMessageComposerVC which allows subclasses to provide the text used for an @-mention within the message text. Currently, the text is restricted to @<user.id>.

This is particularly useful when also subclassing ChatMessageComposerMentionCellView in order to display some custom mention text in the mention composer cell view (as shown below). With this PR, mentionText(forUser:) can be used to match the custom mention composer text.

class CustomChatMessageComposerMentionCellView: ChatMessageComposerMentionCellView {
  
  override func updateContent() {
    usernameLabel.text = content?.name
    if let username  = content?.extraData.username {
      usernameTagLabel.text = "@" + username
    } else {
        usernameTagLabel.text = ""
    }
    avatarView.content = content
  }
  
}

@cjmconie cjmconie changed the title Allow subclasses to customise the mention text Allow subclasses to customise mention text Apr 19, 2021
@codecov
Copy link

codecov bot commented Apr 19, 2021

Codecov Report

Merging #1000 (cb8ef37) into main (b6eb668) will decrease coverage by 0.21%.
The diff coverage is n/a.

❗ Current head cb8ef37 differs from pull request most recent head a353cd5. Consider uploading reports for the commit a353cd5 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1000      +/-   ##
==========================================
- Coverage   89.93%   89.72%   -0.22%     
==========================================
  Files         213      210       -3     
  Lines        8944     8758     -186     
==========================================
- Hits         8044     7858     -186     
  Misses        900      900              
Flag Coverage Δ
llc-tests 89.55% <ø> (-0.22%) ⬇️
llc-tests-ios12 85.85% <ø> (-0.68%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...Chat/WebSocketClient/Events/ConnectionEvents.swift 70.58% <0.00%> (-23.53%) ⬇️
...lient/EventMiddlewares/MemberEventMiddleware.swift 79.16% <0.00%> (-12.94%) ⬇️
...s/StreamChat/WebSocketClient/WebSocketClient.swift 86.54% <0.00%> (-4.10%) ⬇️
Sources/StreamChat/Models/Channel.swift 73.91% <0.00%> (-3.64%) ⬇️
...at/Workers/Background/NewChannelQueryUpdater.swift 87.93% <0.00%> (-0.78%) ⬇️
...lient/Endpoints/Payloads/CurrentUserPayloads.swift 92.30% <0.00%> (-0.55%) ⬇️
.../CurrentUserController/CurrentUserController.swift 94.21% <0.00%> (-0.41%) ⬇️
Sources/StreamChat/Models/Message.swift 88.37% <0.00%> (-0.27%) ⬇️
...eamChat/WebSocketClient/Events/ChannelEvents.swift 93.75% <0.00%> (-0.19%) ⬇️
...rces/StreamChat/Database/DTOs/CurrentUserDTO.swift 98.79% <0.00%> (-0.17%) ⬇️
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b6eb668...a353cd5. Read the comment docs.

@VojtaStavik VojtaStavik self-requested a review April 20, 2021 15:08
@VojtaStavik
Copy link
Contributor

We're in a similar situation here like with #1002. The change you're suggesting looks great at the first glance. Let's please wait a bit until the component audit is finished and then we can finish and merge this PR. Thank you @cjmconie for spending your time on this, too.

@nuno-vieira
Copy link
Member

nuno-vieira commented May 27, 2021

@cjmconie Hello 👋 This is now possible in 4.0-beta by just overriding the mentionSymbol here.
Let us know if you have any questions or have any troubles!

UPDATE: Actually is not possible, I misunderstood the problem. Right now in 4.0-beta, to change this you need to override showMentionSuggestions(). We will keep this in mind and provide an additional API to edit this easily in the next beta versions.

@nuno-vieira nuno-vieira reopened this May 27, 2021
@b-onc
Copy link
Contributor

b-onc commented Jun 17, 2021

Closing this since it's implemented in #1192. This PR will be mentioned in the CHANGELOG. Thanks for the additions @cjmconie !

@b-onc b-onc closed this Jun 17, 2021
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.

None yet

4 participants