Hi,
I have Installed module and place inbox block in sidebar region and configure in such way which displays usersname instead of a latest message(Default).

But when click on the thread link from inbox then it will replace with thread form. Here am I missing something?

Please check attached screenshot.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mayank jeshti created an issue. See original summary.

lamp5’s picture

I can confirm that this issue exists. I have the same problem on branch 8.x-1.x.
Please check this patch or create a similar one to branch 8.x-2.x.

phjou’s picture

phjou’s picture

Status: Active » Needs review
phjou’s picture

Just a precision if you want to reproduce, the bug happens only when the inbox block is before the full thread in the HTML. So if you keep the inbox block in the sidebar with the bartik theme it should work, but if your theme put the sidebar before the content you will probably have the problem.
The patch works well to avoid this behavior.

jlscott’s picture

The patch in #3 does not seem to match the current code, either in beta15 or in dev. I have prepared a new patch that fixes the problem of the replacement thread being inserted at the front of the inbox rather than in the content section where the original full thread was displayed.

phjou’s picture

@jlscott Just tried to apply the patch #3 on the dev branch 8.x-2.x and it applies well.

Your patch applies as well but it doesn't fix this issue when you put the Thread inbox block before the messages. On the other hand, the #3 patch fixes the problem on my environment.

anmolgoyal74’s picture

Can you provide more info.

phjou’s picture

In order to reproduce, you can follow those steps:
- Have a Drupal website with the bartik theme
- Install private_message
- Move the Inbox Block into the breadcrumb region for example (It has to be before the content (the conversation))
- Try to click on a conversation in the Inbox Block to switch
=> The result should be broken by inserting the conversation directly inside the Inbox Block.

anmolgoyal74’s picture

I followed the steps, The conversation switches to the respective thread on clicking on the threads in the Inbox block. What is broken over there?

phjou’s picture

FileSize
34.23 KB
50.25 KB

I just tried again and the bug happens.
Did you move the Inbox block into the breadcrumb region? The inbox HTML must be before the conversation to reproduce the bug.
I just added two screenshots.

PS: sorry for the admin toolbar (this is not the bug)

phjou’s picture

FileSize
109.12 KB

Just uploaded a new screenshot after switching a conversation.

jlscott’s picture

I have rerolled the patch to fix the inbox problem, but because the patch for issue #3041603 is also changing the init() function in file "private_message_thread.js", there is a conflict, so this patch must be applied AFTER the patch for issue #3041603.

phjou’s picture

@jlscott I completely lost you, your new patch doesn't look your old one at all. I will take some time to test your new patch. Could you explain what bug are you trying to fix? I have the feeling that we are trying to fix different things. My first patch is still working good for me and has no conflict with the #3041603 which was pretty complicated.

jlscott’s picture

@phjou. I think I have got myself a bit confused here. I was originally working with release 8.x-2.0-beta15, which turns out to be the final commit on an orphan branch of 8.x-2.x. It contains some code that is not in the current head of 8.x-2.x and the 8.x-2.x HEAD contains code that did not appear in 8.x-2.0-beta15. I recommend that my contributions to this issue are discarded. I have now started using the 8.x-2.x-dev release for the project, and have been able to get it working well with the other patches you are gathering for the next tagged release. Cheers.

phjou’s picture

@jscott Yes I don't really know what happened with the branches. I was confused too, there is an issue about that: Merge latest beta release back into dev branch Thanks for your message :)

phjou’s picture

Ok since the "Load Previous" appears in wrong situations is changing a lots of stuff, I did a new patch if you are using it because it touches the same lines.

So basically if you don't use the patch of #3041603, use the patch #3 otherwise use this new one after the "Load Previous" one.

phjou’s picture

phjou’s picture

anmolgoyal74’s picture

phjou,
Can you share a single patch for all these related issues?

anmolgoyal74’s picture

Status: Needs review » Needs work
phjou’s picture

Ok, I just created a patch that contains all the issues concerning the messaging/thread loading.

Indeed, one patch was not applying after another and a recent commit also created a conflict.

So this patch contains:
- The current issue of this thread
- "Load Previous" appears in wrong situations
- Thread Load previous event listener is added very late

Concerning the issue allowing to remove the style, this is a completely different subject so I will update the patch separately.

Let me know if the new combined patch is working.

phjou’s picture

Status: Needs work » Needs review
FileSize
20.99 KB
zenimagine’s picture

@phjou Thank you for this patch, it fixes a lot of problems but there are errors in the console.

Can this patch be used in production ? Even if there is still work, it fixes bugs that make the module difficult to use with a contrib theme.

phjou’s picture

@zenimagine It's late in Canada, I will check if this patch is responsible for creating the errors in the console later. The patch doesn't change the database or config, so adding or removing the patch should not create any problem but maybe let me some time to check if the modifications are responsible of the errors you gave.
I know that there is an issue with CKEditor that has not been resolved: uncaught exception: The editor instance "edit-message-0-value" is already attached to the provided element.

zenimagine’s picture

@phjou With the patch, the thread is opened correctly on a page, it's already better. I see that <div class="content"> is missing around the thread.
So the block is not updated with ajax and the show more link does not appear.

phjou’s picture

@zenimagine Is it not working with bartik? I'd rather first fix the problems with bartik and then fix the theme dependency issue. We already merged three different problems in that issue, I prefer to not add even more problems in that issue.
Once those are fixed with bartik, fixing the theme dependency will be one of my priority.

  • anmolgoyal74 committed 431b88e on 8.x-2.x authored by phjou
    Issue #3038271 by phjou, jlscott, lamp5, mayank jeshti, zenimagine:...
anmolgoyal74’s picture

Status: Needs review » Fixed

Thanks for the patch.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.