Skip to content

Conversation

benbrown
Copy link

@benbrown benbrown commented Oct 3, 2025

Fixes #5514

Description

This fixes the streaming behavior where new interim messages will replace the old, appear to append the message as it streams in. This is a follow-up to #5517 which initially introduced support for parsing and processing those entities.

However, this small additional change was required for the rendering process to properly find the existing message and update it rather than insert a new message.

Specific Changes

This fix was achieved by capturing the typingActivityId along with the main activityId in the nextActivityToId map such that it can be found the second time around.

In addition, added html2/livestream/entities.*, a selection of tests that demonstrate that the behavior with entities matches the behavior with channelData.

-

  • I have added tests and executed them locally
  • I have updated CHANGELOG.md
  • I have updated documentation

Review Checklist

This section is for contributors to review your work.

  • Accessibility reviewed (tab order, content readability, alt text, color contrast)
  • Browser and platform compatibilities reviewed
  • CSS styles reviewed (minimal rules, no z-index)
  • Documents reviewed (docs, samples, live demo)
  • Internationalization reviewed (strings, unit formatting)
  • package.json and package-lock.json reviewed
  • Security reviewed (no data URIs, check for nonce leak)
  • Tests reviewed (coverage, legitimacy)

OEvgeny
OEvgeny previously approved these changes Oct 14, 2025
'textContent',
'Adipisicing cupidatat eu Lorem anim ut aute magna occaecat id cillum.'
);
expect(pageElements.activityContents()[1]).toHaveProperty('textContent', 'A quick brown fox jumped over');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(pageElements.activityContents()[1]).toHaveProperty('textContent', 'A quick brown fox jumped over');
await waitFor(() => expect(pageElements.activityContents()[1]).toHaveProperty('textContent', 'A quick brown fox jumped over'));

This probably need waitFor() to eliminate test flaky.

'textContent',
'Adipisicing cupidatat eu Lorem anim ut aute magna occaecat id cillum.'
);
expect(pageElements.activityContents()[1]).toHaveProperty('textContent', 'A quick brown fox');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(pageElements.activityContents()[1]).toHaveProperty('textContent', 'A quick brown fox');
await waitFor(() => expect(pageElements.activityContents()[1]).toHaveProperty('textContent', 'A quick brown fox'));

Probably need waitFor() to eliminate test flaky.

Comment on lines +170 to +173
expect(pageElements.activityContents()[1]).toHaveProperty(
'textContent',
'A quick brown fox jumped over the lazy dogs.'
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(pageElements.activityContents()[1]).toHaveProperty(
'textContent',
'A quick brown fox jumped over the lazy dogs.'
);
await waitFor(() =>
expect(pageElements.activityContents()[1]).toHaveProperty(
'textContent',
'A quick brown fox jumped over the lazy dogs.'
)
);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we need to pause the animation, or maybe it is already paused when running in WebDriver.

In case pause is needed, set accessibility "reduced motion" in Dev Tools.

'textContent',
'Adipisicing cupidatat eu Lorem anim ut aute magna occaecat id cillum.'
);
expect(pageElements.activityContents()[1]).toHaveProperty('textContent', 'A quick brown fox jumped over');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(pageElements.activityContents()[1]).toHaveProperty('textContent', 'A quick brown fox jumped over');
await waitFor(() => expect(pageElements.activityContents()[1]).toHaveProperty('textContent', 'A quick brown fox jumped over'));

Comment on lines +177 to +180
expect(currentActivityKeysWithId).toEqual([
[firstActivityKey, ['a-00001']],
[secondActivityKey, ['t-00001', 't-00002', 't-00003']]
]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(currentActivityKeysWithId).toEqual([
[firstActivityKey, ['a-00001']],
[secondActivityKey, ['t-00001', 't-00002', 't-00003']]
]);
await waitFor(() =>
expect(currentActivityKeysWithId).toEqual([
[firstActivityKey, ['a-00001']],
[secondActivityKey, ['t-00001', 't-00002', 't-00003']]
])
);

Comment on lines +211 to +214
expect(pageElements.activityContents()[1]).toHaveProperty(
'textContent',
'A quick brown fox jumped over the lazy dogs.'
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(pageElements.activityContents()[1]).toHaveProperty(
'textContent',
'A quick brown fox jumped over the lazy dogs.'
);
await waitFor(() =>
expect(pageElements.activityContents()[1]).toHaveProperty(
'textContent',
'A quick brown fox jumped over the lazy dogs.'
)
);


// THEN: Should have no typing.
expect(currentActiveTyping).toHaveProperty('u-00001', undefined);
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For completeness, let's try sending another chunk after the livestream is concluded/finalized.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that different than the behavior you'd see with the channelData stream?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You made a good point. Do we have every test against channelData ported to test against entities? Once we deprecated channelData, we will remove those tests and the fortification will be gone.

We can do permutation inside the tests.

Copy link
Contributor

@compulim compulim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented, also few things need to be answered by Eugene.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Livestreaming support through 'entities' activity

3 participants