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] Unintended "Reply on thread" button showing on thread's first message #27716

Merged
merged 11 commits into from Jan 25, 2023

Conversation

Educg550
Copy link
Contributor

Proposed changes (including videos or screenshots)

Referent to TC-128

Before:

image

After:

image

Issue(s)

Steps to test or reproduce

Open a new channel, then:

  1. Send any message
  2. Click "Reply in thread" button and send a message in thread to that message
  3. You'll see that the button "Reply in thread" won't show anymore for the top message, avoiding "thread nesting" and limiting it to only 1 thread, as it should be
  4. Also any other messages sent in that thread won't be allowed to create another inside it

Further comments

As flagged in comment on that issue, seems that there was a discussion whether the thread should open again or have a new created one when clicking on such unintended button. That said, it's conclusive to say that there was no point on opening a new thread for the top message when you're already on one, same for opening new threads for further messages.

@codecov
Copy link

codecov bot commented Jan 10, 2023

Codecov Report

Merging #27716 (dfc490e) into develop (689ee20) will increase coverage by 0.94%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #27716      +/-   ##
===========================================
+ Coverage    41.93%   42.88%   +0.94%     
===========================================
  Files          845      818      -27     
  Lines        17799    17282     -517     
  Branches      2019     1944      -75     
===========================================
- Hits          7464     7411      -53     
+ Misses       10069     9607     -462     
+ Partials       266      264       -2     
Flag Coverage Δ
e2e 42.88% <ø> (+0.94%) ⬆️

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

@hugocostadev
Copy link
Contributor

Hi @Educg550 we have a method called isThreadMainMessage, maybe you can use it to add the correct context and differentiate in other way in the MessageList without creating additional property
image

hugocostadev
hugocostadev previously approved these changes Jan 13, 2023
@hugocostadev hugocostadev added this to the 6.0.0 milestone Jan 16, 2023
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Jan 24, 2023
@Educg550 Educg550 changed the title [FIX] Unintended "Reply on thread" button won't show anymore for any messages inside a thread [FIX] Unintended "Reply on thread" button showing on thread's first message Jan 24, 2023
@kodiakhq kodiakhq bot merged commit 5105b85 into develop Jan 25, 2023
@kodiakhq kodiakhq bot deleted the tc-128 branch January 25, 2023 12:38
@sampaiodiego sampaiodiego mentioned this pull request Feb 17, 2023
@sampaiodiego sampaiodiego mentioned this pull request Mar 9, 2023
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants