claudiu.cristea → created an issue.
Merged. Thank you!
claudiu.cristea → made their first commit to this issue’s fork.
Maybe in few days, not later the end of the year.
A merge request is welcommed
This is a legit request. I see several issue:
- The class (stored in
unread_notification_class
) should be conditionally added. If it comes empty should not be added. - The URL should be passed as var along with from
private_message_page_link
. - The link should be constructed in Twig from the new UR: and
new_items_count
variables - I think
unread_notification_class
is opinionated (the 99+ thingie) and should be deprecated in 4.x and dripped in 5.x
I think you should:
- Configure
private_message_thread
andprivate_message
entity types as group content - Implement hook when a
private_message_thread
andprivate_message
are create, detect the group context, and make those entities belonging to the detected groups. - Alter the routes of
private_message
and change their paths to be under the group path, so that the group context can be determined. E.g.:/private-messages
->/group/{group}/messages
or/private-messages/{private_message_thread}
->/group/{group}/messages/{private_message_thread}
, and so on...
This is not tested but it might work.
Alternatively, you can consider creating a private_message_group
standalone module (or MR to add here as submodule) based the above concept.
Old issue with now recent feedback.
No feedback since 2028. Closing...
I see this is already in place. Unread threads get an unread-thread
class. Closing.
Quoting from 💬 Automatically scroll down to last message when message thread is opened Active :
Why isn't there any "last" class at class="private-message-wrapper" ? How can I add it?
Why isn't there any anchor link to the last message in the thread? How can I add it?This is a more complex issue. If we put a class on the latest we should remove it after the user has seen the message. Say after 3-5 seconds. But if the list is big, some of them are not visible (because you'll need to scroll), so the JS code should be clever enough to remove only the classes form messages that were scrolled into view (with a delay of 3-5 secs). If we still need this feature it should be split in a new ticket
This is doable by placing the inbox block on the desired page (e.g., /private-messages), Feel free to reopen if you think
You can place the block ONLY on /private-messages
and /private-messages/{private_message_thread}
page and you have the inbox. On mobile you might need some JS tweaks to collapse the inbox when you select a thread so that the thread shows on the screen.
Both, 3.0x-dev and 4.x-dev seem to work. But I am not sure.
And I can not install them via composer.
And I will NOT install any dev version on my productive site.
We will release shortly 4.0.0.-alpha1 and 3.0.5 if the -dev are OK for you
Merged to 4.x and 3.0.x. Thank you
Thank you for this catch. The "test only" commit shows the bug. Also, that piece of code looks more simple and easy to read.
We need more info:
- Which was the value of "value" column from the "key_value" table for "collection" = "system.schema" and "name" = "private_message" before you've started the update?
- Which was the initial module version before you've started the update to 3.0.4?
Thank you!
This refactoring will not be backported in 3.0.x
Problems outlined in #5 are fixed. Approving...
Still postponed
This not blocked anymore
Created ✨ Inbox should show the thread from URL in the list Active as a followup
claudiu.cristea → created an issue.
Here's one of the scenarios I've manually tested:
- On a clean install, apart from UID1, I've created 9 regular users (user1, user2, ..., user9)
- As UID1, I've created 9 threads: first with user1 message "THREAD 1", then user2 message "THREAD 2"... and so on.
- Double-checked in the backend to make sure each thread has only one message and a distinct user
- Placed the "Private Message Actions" and "Private Message Inbox" blocks inside the Sidebar region of Olivero theme
- Browse to /private-messages page
- Inbox shows correctly 5 threads. From top to bottom: THREAD 9, THREAD 8, ..., THREAD 5
- The "Load Previous" link is there
- I click "Load Previous" and expecting to append the following batch to the list (THREAD 4..1)
- But only THREAD 4 and THREAD 2 are added
- I wait a little until the Ajax URL https://private-message.ddev.site/private-message/ajax-callback/get_new_... is called
- Ajax inserts also THREAD 3 in the right place
- But THREAD 1 is still missing regadless of how many Ajax refreshes are performed
In parallel, I've installed a new instance with the current 4.x HEAD and there works as it should
@drupalfan2, you didn't answer: Have you tried 3.0.x-dev or 4.x-dev?
I did a review and I left some remarks in MR
Setting correct status
Let's clarify the scope
But we can imagine, if there are several new messages, the user should see, in first place, the oldest of the new ones.
If you have lots of unread messages and scroll to the oldest unread, the reply box will pe pushed down, out of the screen and it makes no sense to focus on that. We can either (a) focus on reply and scroll to latest or (b) scroll to oldest unread and not focus the reply area. I was looking o how popular messengers are working. Every time they are scrolling to latest and are focusing the reply. Let's go with (a)
📌 Support Drupal >=10.3 Active is merged. Unpostponing
Everything goes first to 4.x and, tentatively, to 3.0.x
Thank you for contribution.
Thanks for review. This is not going to be backported to 3.0.x
claudiu.cristea → made their first commit to this issue’s fork.
Fixed latest remarks
Implemented most of remarks and commented to others. Ready for review.
claudiu.cristea → created an issue.
Thank you!
Merged in 4.x. This will NOT be backported to 3.0.x.
Tested also manually, the functionality works correct after the removal of jQuery dependency. Improvement of test assertions is also welcomed. Approving.
Thank you
Thank you for catching this. Could you, please, test the MT?
claudiu.cristea → made their first commit to this issue’s fork.
@drupalfan2, in 3.0.x a lot of JS functionality has been messed up. We're in the middle of stabilizing the code, fixing bugs and ensuring more test coverage. This is an ongoing effort in the new 4.x branch but we're also backporting as much as we can in 3.0.x.
It would help if you can provide a JS test that shows the bug. In the meantime you can test with 4.x and 3.0.x branches maybe this issue is already fixed there.
Thank you for understanding
Postponed on 📌 Support Drupal >=10.3 Active
Ready for review.
Update IS to covers what's in the MR
Unassigning
Assigning
Merged into 4.x and cherry-picked to 3.0.x. Thank you!
This looks good. Agree with the test split, it's more readable and maintainable now.
Assigning for review
claudiu.cristea → created an issue.
claudiu.cristea → created an issue.
Merged into 4.x & 3.0.x. Thank you all
Upgrade path added. This looks good. It has testing coverage and I've tested manually the update path. Approved!
We've opened the 4.x branch. Each new fix/feature should be be filled agains the highest branch. After merging we'll decide whether it makes sense to backport to 3.0.x
Needs an upgrade path to ensure we're also cleaning potential orphan private messages and orphan entries in pm_thread_history table
Great refactoring. Thank you
Closing as duplicate of 📌 Full coverage for JS/Ajax functionality Active .
Should we close this as duplicate of 📌 Full coverage for JS/Ajax functionality Active ?
claudiu.cristea → changed the visibility of the branch 3489458-support-deseralization to hidden.
claudiu.cristea → created an issue.
Nice refactoring. I have only some small remarks and suggestions.
claudiu.cristea → created an issue.
Fix image
Nice Git stats
claudiu.cristea → created an issue.
Postponing on 📌 Full coverage for JS/Ajax functionality Active
claudiu.cristea → created an issue.
OK, set back as Active
Release 3.0.4 is out fixing this regression
Source of bug: random string was used for real user name
Ran several pipelines with this MR and now looks stable.
I'm looking to this
Can I get credit for that?
That's weird because I remember I have checked you (otherwise it would not be added to commit message)