docs(ai-docs): add docs for meetings widget#642
docs(ai-docs): add docs for meetings widget#642riteshfyi wants to merge 16 commits intowebex:nextfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ede33494ce
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b4fdafe3d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| All control `action()` methods take a **destructured object**, not a plain string. | ||
|
|
There was a problem hiding this comment.
This section says every meeting control action() takes a destructured object, but the bundled @webex/sdk-component-adapter implementation used by this package defines positional signatures (for example JoinControl.action(meetingID) and SwitchCameraControl.action(meetingID, cameraID)). Following this guidance will lead maintainers to pass the wrong argument shape and can drop join/device parameters when wiring controls, so the documented call contract should match the actual adapter API.
Useful? React with 👍 / 👎.
mkesavan13creator
added
validated
akulakum
left a comment
akulakum
left a comment
There was a problem hiding this comment.
Thorough Review of ai-docs for Meetings Widget
I've cross-referenced both files (AGENTS.md and ARCHITECTURE.md) against:
- The actual meetings widget source code (
WebexMeetings.jsx,index.js) - The
@webex/sdk-component-adapterimplementation (control action signatures) - The
@webex/componentshooks (useMeetingControl) - The established ai-docs conventions from cc-widgets packages (
station-login,cc-widgets,store,cc-components, etc.)
Overall the documentation is well-written, comprehensive, and clearly structured with excellent Mermaid diagrams and thorough API coverage. However, there are several factual inaccuracies and convention gaps that need addressing.
Summary of Findings
| Category | Count |
|---|---|
| Critical (must fix) | 3 |
| Structural/Convention (should fix) | 6 |
| Content Accuracy (should fix) | 5 |
| Minor (nice to fix) | 6 |
See inline comments for details on each issue.
Structural/Convention Issues (not attached inline)
1. Missing "AI Agent Routing Instructions" section (AGENTS.md)
All existing cc-widget ai-docs for individual widgets include an "AI Agent Routing Instructions" section at the top that directs AI agents to start from the root AGENTS.md. This is missing. Follow the convention from station-login/ai-docs/AGENTS.md.
2. Missing "Installation" section (AGENTS.md)
The cc-widgets ai-docs include an explicit Installation section with yarn add / npm install commands. Should add:
## Installation
yarn add @webex/widgets
3. Missing _Last Updated: YYYY-MM-DD_ footer (both files)
All cc-widgets ai-docs end with _Last Updated: 2025-11-26_. Both files are missing this.
4. Missing **Version:** See [package.json](../package.json) line (AGENTS.md)
The cc-widgets convention includes a version reference in the Overview section.
5. Missing "File Structure" section (ARCHITECTURE.md)
CC-widgets ARCHITECTURE.md files include a directory tree showing the package structure. Should add the packages/@webex/widgets/ tree.
6. Missing "Related Documentation" links to patterns (ARCHITECTURE.md)
CC-widgets ARCHITECTURE.md files include links to relevant patterns files. Currently only links back to its own AGENTS.md.
Minor Issues (not attached inline)
1. Missing @webex/component-adapter-interfaces in Runtime Dependencies table — listed in package.json but not in the doc.
2. Missing prop-types and webex in Peer Dependencies table — package.json lists both as peer deps.
3. Component Table "Source" column points to external repos — Should clarify these are @webex/components package paths, not paths within this repo.
4. Missing documentation of the accessibility workaround code — WebexMeetings.jsx has ~140 lines of accessibility code (componentDidMount) with MutationObserver, focus management, and arrow key navigation. This is significant widget-specific behavior worth mentioning.
5. Missing "Testing" section (ARCHITECTURE.md) — The widget has E2E tests (tests/WebexMeeting.e2e.js) using WebdriverIO but no unit tests. CC-widgets ARCHITECTURE.md files document testing.
6. Widget is a class component — not called out — Unlike cc-widgets (functional components with hooks), this uses a class component with lifecycle methods. This architectural distinction should be noted.
|
|
||
| ### Control Action Parameters | ||
|
|
||
| All control `action()` methods take a **destructured object**, not a plain string. |
There was a problem hiding this comment.
CRITICAL: Control action signatures are WRONG
This entire section is factually incorrect. The statement:
All control
action()methods take a destructured object, not a plain string.
...is the opposite of reality. I verified in the actual @webex/sdk-component-adapter source code — all control action() methods take positional arguments, NOT destructured objects:
| Control | Actual Signature |
|---|---|
AudioControl |
action(meetingID) |
VideoControl |
action(meetingID) |
ShareControl |
async action(meetingID) |
JoinControl |
async action(meetingID) |
ExitControl |
async action(meetingID) |
SwitchCameraControl |
async action(meetingID, cameraID) |
SwitchMicrophoneControl |
async action(meetingID, microphoneID) |
SwitchSpeakerControl |
async action(meetingID, speakerID) |
RosterControl |
async action(meetingID) |
SettingsControl |
action(meetingID) |
Additionally, the useMeetingControl hook in @webex/components calls control.action(meetingID, value) with positional arguments.
The Codex bot also flagged this same issue. This table and the assertion must be corrected to show positional args.
| #### 4. Screen Sharing | ||
|
|
||
| The share button triggers the browser's native screen picker. The SDK handles `getDisplayMedia()` and negotiates the share stream with the backend. | ||
|
|
There was a problem hiding this comment.
Incorrect: SwitchCameraControl.action({ meetingID, cameraId })
This should be SwitchCameraControl.action(meetingID, cameraId) — positional arguments, not a destructured object. The actual adapter implementation uses async action(meetingID, cameraID) with positional params.
Same correction needed for SettingsControl.action() mentioned above.
|
|
||
|
|
||
| --- | ||
|
|
There was a problem hiding this comment.
Missing Peer Dependencies
The package.json lists prop-types: ^15.7.2 and webex: 2.60.2 as peer dependencies, but they're missing from this table. Should add:
| Package | Purpose |
|---|---|
prop-types |
React prop type checking |
webex |
Core Webex SDK (peer) |
Also missing @webex/component-adapter-interfaces from the Runtime Dependencies table above.
|
|
||
| ### Outbound (User Action → Backend) | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Incorrect: Control.action({ meetingID })
The outbound data flow shows:
→ Control.action({ meetingID })
This should be:
→ Control.action(meetingID)
All control action() methods in @webex/sdk-component-adapter use positional arguments, not destructured objects. The useMeetingControl hook calls control.action(meetingID, value) with positional params.
| This is the real shape emitted by `adapter.meetingsAdapter.getMeeting(ID)`: | ||
|
|
||
| ``` | ||
| { |
There was a problem hiding this comment.
Missing 'ASKING' permission state
The localAudio.permission and localVideo.permission fields document:
permission: string | null // 'ALLOWED' | 'ERROR' | null
But the widget's actual render logic (WebexMeetings.jsx lines 188-191) critically depends on the 'ASKING' state:
if (audioPermission === 'ASKING') {
content = <WebexMediaAccess meetingID={meeting.ID} media="microphone" ... />;
} else if (videoPermission === 'ASKING') {
content = <WebexMediaAccess meetingID={meeting.ID} media="camera" ... />;
}Should be: // 'ALLOWED' | 'ASKING' | 'ERROR' | null
| participant Backend | ||
|
|
||
| User->>Component: Mount widget with accessToken | ||
| Component->>SDK: Webex.init({ credentials: { access_token } }) |
There was a problem hiding this comment.
Inaccurate: Webex.init()
The sequence diagram shows:
Component->>SDK: Webex.init({ credentials: { access_token } })
But the actual source code (WebexMeetings.jsx line 260) uses the constructor form:
const webex = new Webex({ credentials: { access_token: props.accessToken }, ... });Should be new Webex({ credentials: { access_token } }) — this also appears in the Three-Repository Architecture table in AGENTS.md (line 168) where it says Webex.init() under Key Exports Used.
| Note over User: User selects a different camera | ||
|
|
||
| User->>Component: Select new camera from dropdown | ||
| Component->>Adapter: SwitchCameraControl.action({ meetingID, cameraId }) |
There was a problem hiding this comment.
Incorrect: SwitchCameraControl.action({ meetingID, cameraId })
Same issue as in AGENTS.md — should use positional args:
Component->>Adapter: SwitchCameraControl.action(meetingID, cameraId)
Not:
Component->>Adapter: SwitchCameraControl.action({ meetingID, cameraId })
|
|
||
| - React strict mode causing double initialization | ||
| - Missing cleanup on prop changes | ||
| - Missing dependency array in useEffect |
There was a problem hiding this comment.
Troubleshooting #5 references useEffect but widget is a class component
This troubleshooting item mentions:
- "Missing dependency array in useEffect"
- "Implement proper cleanup in useEffect return"
But WebexMeetingsWidget is a class component — it uses componentDidMount and componentWillUnmount, not hooks. These references to useEffect would confuse developers. Either:
- Rewrite to reference class lifecycle methods, or
- Clarify this applies to consumer/wrapper code, not the widget itself.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a32332f38
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5baf0e3727
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| Do **not** use this file as your main entry point for reasoning or code generation. | ||
|
|
||
| - **First step:** Locate and begin with the closest parent `AGENTS.md` file in the `ai-docs` directory structure. For example, the root `ai-docs/AGENTS.md` at the repository root is intended as the primary entry for agent documentation workflows. |
There was a problem hiding this comment.
The first-step guidance tells agents to start from ai-docs/AGENTS.md at repository root, but this commit’s tree only contains a root-level AGENTS.md (no ai-docs/AGENTS.md). Following this instruction sends automation to a non-existent file and skips the actual top-level guidance, so the path needs to be corrected to the real root AGENTS location.
Useful? React with 👍 / 👎.
|
|
||
| - `WebexInterstitialMeeting` renders when `state === 'NOT_JOINED'` | ||
| - Controls available pre-join: `mute-audio`, `mute-video`, `settings`, `join-meeting` | ||
| - `JoinControl.display()` shows a hint like "Unmuted, video on" based on current state |
There was a problem hiding this comment.
can u point me to code block where this is ? i dont see this in code base
There was a problem hiding this comment.
There was a problem hiding this comment.
|
|
||
| **Key Points:** | ||
|
|
||
| - `SettingsControl.action()` opens the `WebexSettings` modal |
There was a problem hiding this comment.
There was a problem hiding this comment.
|
Props include theme and featureFlags │ ❌ Wrong │ These do not exist in propTypes. theme is applied externally via CSS │ |
|
│ ../../../ai-docs/patterns/web-component-patterns.md │ ❌ Broken link — file does not exist │ |
|
├────────────────────────────────┼────────────┼────────────────────────────────────────────────────────────────────────────────┤ |
Removed the invalid Reference |
Added a Line for that |
1.Do not see any referrence of theme & featureFlags parameters in the docs. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc7f57a8be
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| The widget handles SDK initialization, adapter creation, meeting creation, and all internal wiring via the `withAdapter` and `withMeeting` HOCs. Consumers just import and render with props: | ||
|
|
||
| ```jsx | ||
| import {WebexMeetingsWidget} from '@webex/widgets'; |
There was a problem hiding this comment.
In the basic integration path, this example only imports WebexMeetingsWidget, but packages/@webex/widgets/README.md requires consumers to also load @webex/widgets/dist/css/webex-widgets.css. Agents or developers following this snippet will produce an unstyled meetings UI, which is a broken setup for any fresh embed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
the CSS is self-contained within the widget's source imports.
| ```bash | ||
| # Install as a standalone package | ||
| yarn add @webex/widgets | ||
| npm install @webex/widgets |
There was a problem hiding this comment.
For a fresh application, yarn add @webex/widgets / npm install @webex/widgets is incomplete: packages/@webex/widgets/package.json declares react, react-dom, prop-types, and webex as peer dependencies, and the widget imports webex from the host app. Following this block leaves at least webex unresolved unless the consumer happens to install it separately, so the documented install flow will fail out of the box.
Useful? React with 👍 / 👎.
| ├── tests/ | ||
| │ ├── WebexMeetings/ | ||
| │ │ └── WebexMeetings.test.jsx # Unit tests |
There was a problem hiding this comment.
This file tree advertises a Jest-based unit-test setup (tests/WebexMeetings/WebexMeetings.test.jsx and, later in the tree, jest.config.js), but this package only contains the WDIO E2E suite under tests/WebexMeeting.e2e.js. Anyone using these ai-docs to add or debug tests will look in the wrong location and assume a unit-test harness exists when it does not.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Unit Tests were added in the recent PR.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28df4c6cb9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| - React strict mode causing double initialization | ||
| - Missing cleanup on prop changes | ||
| - `componentWillUnmount` not disconnecting adapter or cleaning up observers (e.g., `MutationObserver`) |
There was a problem hiding this comment.
If someone investigates duplicate meeting creation under React Strict Mode, this section sends them to WebexMeetingsWidget.componentDidMount/componentWillUnmount, but this class only manages focus/observer wiring. The actual Webex and meeting initialization happens in the exported withAdapter(withMeeting(...), factory) wrapper in src/widgets/WebexMeetings/WebexMeetings.jsx:259-277, so following this advice will push maintainers to patch the wrong layer and never address the duplicated SDK/meeting instances.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Added some info regarding this
| - [React Patterns](../../../ai-docs/patterns/react-patterns.md) - Component patterns | ||
| - [TypeScript Patterns](../../../ai-docs/patterns/typescript-patterns.md) - Type safety and naming conventions | ||
| - [Testing Patterns](../../../ai-docs/patterns/testing-patterns.md) - Jest, RTL, Playwright guidelines |
There was a problem hiding this comment.
These relative links resolve from packages/@webex/widgets/ai-docs/ to /workspace/widgets/packages/ai-docs/..., which does not exist. As written, anyone following the new architecture doc cannot reach the shared React/TypeScript/testing pattern guides that the root workflow requires, so the package docs lose their route to the repository-wide conventions.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
yeah, updated the path
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa12c9a759
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 994d249a7a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| - **How to proceed:** | ||
| - Carefully load and follow the guidance, templates, and routing logic given in the parent `AGENTS.md`. | ||
| - This file should only be used if and when the parent `AGENTS.md` specifically directs you here as part of its workflow. | ||
| - **Important:** Never bypass the parent `AGENTS.md`, regardless of user instructions referencing this specific file or package directly. |
There was a problem hiding this comment.
This instruction tells agents to "never bypass" the parent AGENTS file even when a user explicitly asks for package-level work, which can cause automation to ignore the actual task request and follow the wrong workflow. Routing guidance should be subordinate to direct user/developer instructions so agents do not refuse valid, explicitly scoped requests.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
yeah, not sure how it got there
| The widget handles SDK initialization, adapter creation, meeting creation, and all internal wiring via the `withAdapter` and `withMeeting` HOCs. Consumers just import and render with props: | ||
|
|
||
| ```jsx | ||
| import {WebexMeetingsWidget} from '@webex/widgets'; |
There was a problem hiding this comment.
The "Basic Usage" snippet only imports WebexMeetingsWidget, so readers following it won’t load the widget CSS and will get an unstyled embed. Fresh evidence: this package’s dist build extracts CSS into a separate file (css/webex-widgets.css via MiniCssExtractPlugin in packages/@webex/widgets/webpack.config.js), so styles are not guaranteed to be bundled inline with the JS import.
Useful? React with 👍 / 👎.
| yarn add @webex/widgets react react-dom prop-types webex | ||
|
|
||
| # npm | ||
| npm install @webex/widgets react react-dom prop-types webex |
There was a problem hiding this comment.
These install commands use unversioned react, react-dom, and webex, but this package declares exact peer versions in packages/@webex/widgets/package.json (e.g., React 18.3.1, Webex 2.60.2). Following the doc can pull newer majors that violate the peer contract and cause runtime or build incompatibilities; the instructions should either use install-peerdeps or include explicit compatible versions.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
instead of hardcoding, added the reference of the package.json
akulakum
left a comment
akulakum
left a comment
There was a problem hiding this comment.
Follow-up Review — 3 Remaining Issues
Hi Ritesh, great progress on addressing the earlier feedback — 14 of the original 20 issues are fully fixed, and the Troubleshooting #5 rewrite is excellent. However, 3 issues remain that block approval, all stemming from the same root cause:
Issue 1: action({ meetingID }) should be action(meetingID) — 18 occurrences across both files
The original critical issue about control action signatures is only partially fixed. The Control Action Parameters table was cleaned up (good), but:
- AGENTS.md line ~283 still says "receives a destructured object" (should say "positional arguments")
- AGENTS.md lines ~115-116 still show
SettingsControl.action({ meetingID })andSwitchCameraControl.action({ meetingID, cameraId }) - ARCHITECTURE.md line ~174 data flow still shows
Control.action({ meetingID }) - ARCHITECTURE.md sequence diagrams — 15 occurrences across diagrams 3-10 all use
action({ meetingID })instead ofaction(meetingID)
Fix: Global find-and-replace action({ meetingID }) → action(meetingID) and action({ meetingID, cameraId }) → action(meetingID, cameraId) across both files. Also fix the description at line ~283.
Issue 2: AI Agent Routing path is wrong
The routing section references ai-docs/AGENTS.md at the repo root, but that file doesn't exist. The actual file is ./AGENTS.md at the repo root.
Issue 3: action({ meetingID, meetingPasswordOrPin, participantName }) in Join sequence diagram (line ~329 in ARCHITECTURE.md)
This should be action(meetingID) — JoinControl.action() only takes meetingID as a positional arg. The password/name are handled via meeting state, not passed through action().
Quick Fix Guide
All 19 changes are the same pattern — just remove curly braces:
action({ meetingID })→action(meetingID)action({ meetingID, cameraId })→action(meetingID, cameraId)action({ meetingID, meetingPasswordOrPin, participantName })→action(meetingID)- "receives a destructured object" → "is invoked with positional arguments
(meetingID, value)" ai-docs/AGENTS.md→AGENTS.md
This should take ~15 minutes. Once these are fixed, I'm happy to approve.
| **Source:** [`@webex/sdk-component-adapter`](https://github.com/webex/sdk-component-adapter) → [`src/MeetingsSDKAdapter.js`](https://github.com/webex/sdk-component-adapter/blob/master/src/MeetingsSDKAdapter.js) | ||
|
|
||
|
|
||
| | Method | Parameters | Returns | Description | |
There was a problem hiding this comment.
Still incorrect. This says:
Each control's
action()receives a destructured object from theuseMeetingControlhook
The useMeetingControl hook calls control.action(meetingID, value) with positional arguments, not a destructured object. See @webex/components source:
return control ? [function (value) {
return control.action(meetingID, value);
}, display] : [function () {}, {}];Suggested fix:
Each control's
action()is invoked by theuseMeetingControlhook with positional arguments(meetingID, value)and calls the corresponding adapter method internally.
There was a problem hiding this comment.
misconception of the codex bot
|
|
||
| **Key Points:** | ||
|
|
||
| - `WebexInterstitialMeeting` renders when `state === 'NOT_JOINED'` |
There was a problem hiding this comment.
Still not fixed from original review. These two lines still use destructured object notation:
- `SettingsControl.action({ meetingID })` opens the `WebexSettings` modal
- `SwitchCameraControl.action({ meetingID, cameraId })` calls `switchCamera(meetingID, cameraId)` on the adapter
Should be:
- `SettingsControl.action(meetingID)` opens the `WebexSettings` modal
- `SwitchCameraControl.action(meetingID, cameraId)` calls `switchCamera(meetingID, cameraId)` on the adapter
There was a problem hiding this comment.
misconception of the codex reiview bot
| Do **not** use this file as your main entry point for reasoning or code generation. | ||
|
|
||
| - **First step:** Locate and begin with the closest parent `AGENTS.md` file in the `ai-docs` directory structure. For example, the root `ai-docs/AGENTS.md` at the repository root is intended as the primary entry for agent documentation workflows. | ||
| - **How to proceed:** |
There was a problem hiding this comment.
Path is wrong. This references ai-docs/AGENTS.md at the repository root, but that file does not exist. The actual root-level agent file is ./AGENTS.md (at the repo root, not inside an ai-docs/ subfolder).
The Codex bot also flagged this same issue.
Suggested fix:
First step: Locate and begin with the root
AGENTS.mdat the repository root. It is intended as the primary entry for agent documentation workflows.
There was a problem hiding this comment.
every tiny mistake, will change
| User clicks control button | ||
| → Component (WebexMeetingControl) | ||
| → useMeetingControl hook | ||
| → Control.action({ meetingID }) |
There was a problem hiding this comment.
Still not fixed from original review. The outbound data flow shows:
→ Control.action({ meetingID })
Should be:
→ Control.action(meetingID)
All control action() methods use positional arguments.
| participant Backend | ||
|
|
||
| User->>Component: Click "Join Meeting" button | ||
| Component->>Adapter: action({ meetingID }) |
There was a problem hiding this comment.
Still not fixed. This and 14 other sequence diagrams below still use action({ meetingID }) notation.
This specific line is also wrong in a second way — JoinControl.action() only takes meetingID as a positional arg, not { meetingID, meetingPasswordOrPin, participantName }. The password/name are handled via meeting state in the adapter, not passed through action().
Should be:
Component->>Adapter: action(meetingID)
Full list of sequence diagram lines that need the same fix:
- Line ~329 (Join):
action({ meetingID, ... })→action(meetingID) - Line ~371, ~385 (Mute Audio):
action({ meetingID })→action(meetingID) - Line ~414, ~428 (Video):
action({ meetingID })→action(meetingID) - Line ~453, ~469 (Screen Share):
action({ meetingID })→action(meetingID) - Line ~495, ~503 (Roster):
action({ meetingID })→action(meetingID) - Line ~524, ~534, ~543 (Settings/Camera): same fix, also
action(meetingID, cameraId)for SwitchCamera - Line ~571 (Leave):
action({ meetingID })→action(meetingID) - Line ~605, ~625 (Auth):
action({ meetingID })→action(meetingID)
akulakum
left a comment
akulakum
left a comment
There was a problem hiding this comment.
Good work overall, @riteshfyi. The documentation is comprehensive and well-structured.
-
action({ meetingID }) → action(meetingID) — 18 occurrences across both files (AGENTS.md use cases, AGENTS.md control action description, ARCHITECTURE.md data flow, and all sequence diagrams). Global find-and-replace to remove the curly braces. Also fix action({ meetingID, cameraId }) → action(meetingID, cameraId) and action({ meetingID, meetingPasswordOrPin, participantName }) → action(meetingID).
-
AGENTS.md line ~283 — Change "receives a destructured object" to "is invoked with positional arguments (meetingID, value)".
-
AGENTS.md AI Agent Routing (line ~10) — Change ai-docs/AGENTS.md to AGENTS.md (the root file is at ./AGENTS.md, not ai-docs/AGENTS.md).
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2cf590ba11
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| ## Installation | ||
| The widget declares `react`, `react-dom`, `prop-types`, and `webex` as **peer dependencies**. Consumers must install them alongside the widget at the versions specified in [package.json](../package.json) under `peerDependencies`. | ||
| ```bash |
There was a problem hiding this comment.
The installation snippet opens a fenced block with ```````bashbut never closes it, so everything after line 339 (including theAdditional Resources` section and link to `ARCHITECTURE.md`) is rendered as code instead of documentation. This breaks navigation and makes the rest of the page easy to miss for contributors following these ai-docs.
Useful? React with 👍 / 👎.
4 participants
COMPLETES #https://jira-eng-gpk2.cisco.com/jira/browse/SPARK-783362
This pull request addresses
adds ai-docs for meetings widget repo
by making the following changes
created AGENTS.md & ARCHITECTURE.md
Change Type
The following scenarios were tested
< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >
The GAI Coding Policy And Copyright Annotation Best Practices
Checklist before merging
Make sure to have followed the contributing guidelines before submitting.