-
Notifications
You must be signed in to change notification settings - Fork 55
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(webexMember): updated member name handling #845
Conversation
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant Component as WebexMember
participant Member as Member Object
Component->>Member: Retrieve member details
Member-->>Component: Return member data
Component->>Component: Set memberName = member.displayName or default
Component->>Component: Render memberName in JSX
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/components/WebexMember/WebexMember.jsx (2)
67-67
: LGTM! Consider minor accessibility enhancement.The name display logic handles all states appropriately (loading, error, success). However, consider using
<em>
instead of<i>
for better semantic meaning and accessibility.- {(memberName ?? <Spinner size={18} />) || <i>Name not available</i>} + {(memberName ?? <Spinner size={18} />) || <em>Name not available</em>}
42-42
: Well-structured name handling improvements.The introduction of
memberName
and its associated display logic is a good architectural improvement that:
- Centralizes name resolution logic
- Properly handles loading and error states
- Maintains backward compatibility
Consider documenting these name resolution rules in the component's JSDoc comments for future maintainers.
Also applies to: 67-67
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/components/WebexMember/WebexMember.jsx
(2 hunks)
🔇 Additional comments (1)
src/components/WebexMember/WebexMember.jsx (1)
42-42
: Verify member.name property and add null check for displayName.
While the optional chaining for member?.name
is good, consider adding a null check for displayName
as well. Also, verify that name
is the correct property to access from the member object.
Let's verify the member object structure:
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.
Let us get the other PR in first and then merge this.
Sure @adhmenon first this sdk-component-adpater PR will go first |
## [1.275.3](v1.275.2...v1.275.3) (2024-11-13) ### Bug Fixes * **webexMember:** updated member name handling ([#845](#845)) ([881ddae](881ddae))
🎉 This PR is included in version 1.275.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This pull request addresses
Issue:- The user is trying to enter the name according to their wish but whatever the name entered by the user unfortunately it not going to display on the sample app when you click to see in the participants list means it taking the system default name so for this issue the following changes are done
by making following changes
This PR updates the logic for displaying the member's name in the
WebexMember
component. If themember.name
is unavailable, it will fall back to thedisplayName
. This ensures more consistent name handling within the component.sdk-component.recording.mp4
This below recording for the system default
displayName
when you are not providing the name that means whenever the name field is empty and not provides anything then the display name will system default name
displayname.system.default.mp4