-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Fix sorting activities when they are grouped in either part grouping or livestreaming #5635
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this order?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't use livestreaming so the question remains
packages/api/src/providers/GroupActivities/private/createDefaultGroupActivitiesMiddleware.ts
Show resolved
Hide resolved
| directLine.emulateIncomingActivity(withPosition(activities.at(2), 3, 'one updated to three')); | ||
|
|
||
| // Then: show all activities repositioned according to new positions | ||
| // Then: show the "one" activity moved before "foru" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // Then: show the "one" activity moved before "foru" | |
| // Then: show the "one" activity moved before "four" |
Changelog Entry
Fixed
Description
When receiving activities, they are sorted with an insertion sort algorithm. However, the algorithm did not handle grouping. Thus, activities in the same group could be separated as a new activity inserted in-between them.
This pull request will treat activity group as a unit.
Design
Logical timestamp
The technical definition of logical timestamp:
The order is based on reliability of the value:
Grouping
Currently, an activity can be grouped or ungrouped. There are 2 types of grouping for grouped activities:
HowTo/HowToStep(a.k.a. chain of thoughts)The order of grouping should be:
For example, given a part grouping contains 1 livestream session and 1 ungrouped, when sorted:
Activities should be group-sorted by livestream before part grouping.
Sorting algorithm
Grouping and ungrouped activities are of their own unit. For example,
Then, the units are sorted by their corresponding logical timestamp.
For ungrouped activities, their logical timestamp is of their own.
For grouped activities, their logical timestamp is:
For example, for part grouping with livestream:
The logical timestamp of this part grouping will be 1_500.
What if 2 activities share the same timestamp?
As activities are sorted when inserted, their position will be fixed once they are inserted.
Technically, the sorting comparer is as follows:
In plain English, "the inserting activity will be inserted right before the activity with a larger logical timestamp."
Why livestream session only look at logical timestamp of 1th and Nth revision but not 2nd...N-1th?
This is because livestream session are updated very frequently. If we look at 2nd...N-1th, the activity could be constantly move to the bottom of chat history.
In a simultaneous scenario where 2 livestream sessions are continuously updating, both sessions will "race" their position to the bottom and cause a lot of "jumps". This is not desirable in UX term.
Therefore, to reduce jumpiness, we are moving livestream sessions only when they are first opened or finalized.
New Redux reducer
groupedActivitiesNew
groupedActivitiesreducer is introduced to add internal states that are required outside of what currentactivitiesreducer provides.For
activitiesreducer, we kept the interface the same by copying part of the result fromgroupedActivities. This is done by a customcombineReducersfunction. New hooks or selectors need to be changed.Specific Changes
groupedActivitiesreduceractivitiesreducer will reuse the result fromgroupedActivitiesreducer thru a customcombineReducersfunctionCHANGELOG.mdReview Checklist
Accessibility reviewed (tab order, content readability, alt text, color contrast)Browser and platform compatibilities reviewedCSS styles reviewed (minimal rules, noz-index)Internationalization reviewed (strings, unit formatting)package.jsonandpackage-lock.jsonreviewedSecurity reviewed (no data URIs, check for nonce leak)