Skip to content

feat(WebexMember): split component into space members and meeting participant - #772

Open
thescripted wants to merge 2 commits into
webex:masterfrom
thescripted:refactor/split-webex-member
Open

feat(WebexMember): split component into space members and meeting participant#772
thescripted wants to merge 2 commits into
webex:masterfrom
thescripted:refactor/split-webex-member

Conversation

@thescripted

@thescriptedthescripted commented Mar 22, 2022

Copy link
Copy Markdown
Contributor

Separate WebexMember intoWebexMember for spaces and WebexMeetingParticipant for meetings
Separate WebexMemberRoster intoWebexMemberRoster for spaces and WebexMeetingRoster for meetings

SPARK-308491
SPARK-308492

@thescripted
thescriptedforce-pushed the refactor/split-webex-member branch from 473bc67 to 78b320fCompareMarch 22, 2022 19:45

@taymoork2taymoork2 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

Comment threadsrc/components/WebexMeetingParticipant/WebexMeetingParticipant.jsx Outdated
Comment threadsrc/components/WebexMeetingParticipant/WebexMeetingParticipant.jsx Outdated
Comment threadsrc/components/WebexMember/WebexMember.jsx
@thescripted
thescriptedforce-pushed the refactor/split-webex-member branch 4 times, most recently from f1a0714 to 6991e6eCompareMarch 30, 2022 14:59
@thescripted
thescriptedforce-pushed the refactor/split-webex-member branch from 6991e6e to 6827327CompareMarch 30, 2022 18:32
@cipak

Copy link
Copy Markdown
Contributor

Separate WebexMember intoWebexMember for spaces and WebexMeetingParticipant for meetings Separate WebexMemberRoster intoWebexMemberRoster for spaces and WebexMeetingRoster for meetings

SPARK-308491SPARK-308492

This seems to introduce some code duplication, can this be avoided?

For example, leaving the generic WebexMember component as-is, and introducing two additional components:

  • WebexSpaceMember
  • WebexMeetingMember

These components would just use WebexMember with a hardcoded destination type, avoiding of duplication of js code and css:

functionWebexSpaceMember(...){return<WebexMemberdestinationType="ROOM".../>;
}

Maybe the same could be done for the roster. I would suggest using the names WebexSpaceRoster and WebexMeetingRoster, so that the difference is clear: SpaceRoster vs MeetingRoster, instead of MemberRoster vs MeetingRoster which is a bit confusing.

Sign up for freeto 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.

4 participants

@thescripted@cipak@taymoork2@lalli-flores