Problem/Motivation

A common usage of Private Message is to display the Inbox block in a sidebar and the selected thread in the main content.

In these conditions, when the main page "/private-messages" is open, the first thread in the Inbox will be displayed in the main content. Thus, the first thread in the Inbox should be highlighted and for that it should have the class "active-thread". But this is not happening.

Steps to reproduce

I'm not sure this can be reproduced in all environments.
For me (besides the Drupal site I'm working) this only happens when BigPipe is enabled! So, first step is to ensure this module is enabled.
Display the Inbox block in a sidebar and the selected thread in the main content.
Go to "/private-messages".
Check that the first thread in the Inbox is not highlighted. Also inspect the code and check that the class "active-thread" is missing in the element's <div>.

The problem is that the code in "private_message_thread.js depends on the code in "private_message_inbox_block.js" for this to work correctly.
This can be seen here: https://git.drupalcode.org/project/private_message/-/blob/3.0.2/js/priva...
If Drupal.PrivateMessages.setActiveThread defined in "inbox_block_script" does not exist, Drupal.PrivateMessages.setActiveThread(originalThreadId); is not running.

Proposed resolution

We should isolate better the responsibilities. I propose to remove the "setActiveThread" calls from "private_message_thread.js".
We should also be more careful to not rely in HTML elements that may still not be there when the JS runs. That was the case for the variable "container" in "private_message_inbox_block.js".

Issue fork private_message-3486915

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

dxvargas created an issue. See original summary.

dxvargas’s picture

dxvargas commented
Issue summary: View changes
dxvargas’s picture

dxvargas commented
Issue summary: View changes

dxvargas opened merge request !137

dxvargas’s picture

dxvargas commented
Status: Active » Needs review

For now I've implementing the proposed resolution and I'll wait for feedback. Moving to Needs review.

Some remarks:

  1. Maybe Drupal.PrivateMessages.loadThread() could have as second argument the code to run on success? With the changes I'm proposing in the MR we are not waiting for the AJAX result, to highlight the new thread in Inbox. But I actually think like this is the right way and also the full thread should be replaced with a timer while the new one is being loaded.
  2. Maybe a better solution would be to implement a MutationObserver logic. Like that the Inbox block would automatically react to changes in the full thread.
  3. The method setActiveThread is enough for the module. For now I'm keeping Drupal.PrivateMessages.setActiveThread just for BC for third parties that may rely on this method.
saidatom’s picture

saidatom
Location Lisbon
commented
Status: Needs review » Reviewed & tested by the community

Great! Looks good, moving to RTBC.

claudiu.cristea made their first commit to this issue’s fork.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Reviewed & tested by the community » Fixed

Thank you!

Status: Fixed » Closed (fixed)

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