Skip to content
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

Fix: avatar display in various scenarios #5346

Merged
merged 5 commits into from
Nov 8, 2024
Merged

Fix: avatar display in various scenarios #5346

merged 5 commits into from
Nov 8, 2024

Conversation

OEvgeny
Copy link
Collaborator

@OEvgeny OEvgeny commented Oct 31, 2024

Fixes #5317

Changelog Entry

  • Improved avatar display and grouping behavior by fixing rendering issues and activity sender identification, in PR #5346, by @OEvgeny

Description

This PR addresses two issues affecting avatar display in Web Chat:

  1. Avatar rendering inconsistencies caused by component updates
  2. Incorrect grouping of activities leading to mismatched avatar displays

Design

The solution implements two key improvements:

  1. Direct Avatar Rendering

    • Replaced FixedWidthImage component with native img element
    • Added proper aspect ratio and object-fit handling
    • Improved overflow handling using modern CSS properties
  2. Enhanced Activity Grouping

    • Added sender ID check in grouping logic
    • Optimized grouping algorithm performance

Specific Changes

  • Updated ImageAvatar.tsx:
    • Replaced FixedWidthImage with optimized img element
    • Improved image overflow handling
  • Updated createDefaultGroupActivitiesMiddleware.ts:
    • Added shouldGroupSender function to check both role and ID
    • Optimized bin function using for...of
  • Updated ImageAvatar.ts styles:
    • Added aspect-ratio property
    • Updated overflow handling with fallback (for IE)
  • Updated createCoreMiddleware.tsx:
    • Added CSS overflow fallback for better browser compatibility (IE)

-

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

Review Checklist

  • 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 marked this pull request as ready for review November 1, 2024 02:32
@OEvgeny
Copy link
Collaborator Author

OEvgeny commented Nov 7, 2024

Here is the difference between npm@main and the branch on the introduced test activity/grouping.multipleAvatars with disabled grouping by timestamp for cleaner diff:
image

@OEvgeny OEvgeny force-pushed the fix/avatar-display branch from 8b2ea0e to decfd47 Compare November 7, 2024 19:33
@OEvgeny OEvgeny requested a review from compulim November 7, 2024 21:55
@OEvgeny OEvgeny merged commit 2fc4f5c into main Nov 8, 2024
27 checks passed
@OEvgeny OEvgeny deleted the fix/avatar-display branch November 8, 2024 16:04
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.

Large bot avatar image are not showing
2 participants