Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
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.
Comment | File | Size | Author |
---|---|---|---|
#24 | Capture d’écran du 2019-10-02 06-34-22.png | 338.24 KB | zenimagine |
#23 | private_message-3038271_3041603_3046653-23.patch | 20.99 KB | phjou |
#17 | private_message-load_messages_from_inbox_to_main_thread_error-3038271-17.patch | 501 bytes | phjou |
#13 | private_message-3038771-inbox-not-working-13.patch | 501 bytes | jlscott |
#12 | pv_after2.png | 109.12 KB | phjou |
Comments
Comment #2
lamp5I 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.
Comment #3
phjouJust did the patch for 8.x-2.x
Comment #4
phjouComment #5
phjouJust 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.
Comment #6
jlscott CreditAttribution: jlscott as a volunteer and at Xequals for SpaceBase commentedThe 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.
Comment #7
phjou@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.
Comment #8
anmolgoyal74 CreditAttribution: anmolgoyal74 at OpenSense Labs for DrupalFit commentedCan you provide more info.
Comment #9
phjouIn 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.
Comment #10
anmolgoyal74 CreditAttribution: anmolgoyal74 at OpenSense Labs for DrupalFit commentedI followed the steps, The conversation switches to the respective thread on clicking on the threads in the Inbox block. What is broken over there?
Comment #11
phjouI 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)
Comment #12
phjouJust uploaded a new screenshot after switching a conversation.
Comment #13
jlscott CreditAttribution: jlscott as a volunteer and at Xequals for SpaceBase commentedI 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.
Comment #14
phjou@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.
Comment #15
jlscott CreditAttribution: jlscott as a volunteer and at Xequals for SpaceBase commented@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.
Comment #16
phjou@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 :)
Comment #17
phjouOk 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.
Comment #18
phjouComment #19
phjouComment #20
anmolgoyal74 CreditAttribution: anmolgoyal74 at OpenSense Labs for DrupalFit commentedphjou,
Can you share a single patch for all these related issues?
Comment #21
anmolgoyal74 CreditAttribution: anmolgoyal74 at OpenSense Labs for DrupalFit commentedComment #22
phjouOk, 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.
Comment #23
phjouComment #24
zenimagine CreditAttribution: zenimagine commented@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.
Comment #25
phjou@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.
Comment #26
zenimagine CreditAttribution: zenimagine commented@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.
Comment #27
phjou@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.
Comment #29
anmolgoyal74 CreditAttribution: anmolgoyal74 at OpenSense Labs for DrupalFit commentedThanks for the patch.