docs(samples): unify the widgets sample apps #363
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request introduces significant changes to the project's workspace structure, build scripts, and sample applications. The modifications include renaming sample applications, updating build and serve commands, introducing a new meeting sample app, and reorganizing the directory structure. The changes aim to streamline the development workflow and provide more flexible sample applications for Webex widgets. Changes
Sequence DiagramsequenceDiagram
participant User
participant App
participant WebexMeetingsWidget
User->>App: Enter Access Token
User->>App: Select Meeting Options
App->>WebexMeetingsWidget: Configure Widget
WebexMeetingsWidget-->>App: Widget Initialized
User->>WebexMeetingsWidget: Interact with Meeting Controls
Possibly related PRs
Suggested reviewers
Poem
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 (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
packages/@webex/widgets/package.json (1)
⚠️ Suspicious React version detected - recommend immediate updateThe specified React version (18.3.1) shows concerning signs of being potentially unofficial or compromised:
- Missing publication date
- Not listed in any official distribution tags
- Unusual version number between stable releases
Recommend updating to either:
- React 18.2.0 (latest stable in 18.x series)
- React 19.0.0 (current stable release)
🔗 Analysis chain
Line range hint
1-150: Verify React version compatibility.The specified React version (18.3.1) seems unusual as the latest stable version is 18.2.0.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if React version exists curl -s https://registry.npmjs.org/react | jq '.versions | has("18.3.1")' # List latest React versions for reference curl -s https://registry.npmjs.org/react | jq -r '.["dist-tags"]'Length of output: 391
Script:
#!/bin/bash # Get details about React version 18.3.1 curl -s https://registry.npmjs.org/react/18.3.1 | jq '{ version: .version, published: .time, description: .description, dist: .dist }'Length of output: 808
🧹 Nitpick comments (8)
widgets-samples/samples-meeting-app/src/App.jsx (2)
12-14: Consider consolidating token state.Currently maintaining two separate states (
tokenInputandtoken) for the same data. Consider using a single state with an additional boolean flag for editing mode.- const [tokenInput, setTokenInput] = useState(''); - const [token, setToken] = useState(); + const [token, setToken] = useState(''); + const [isEditing, setIsEditing] = useState(true);
66-71: Enhance button accessibility.The buttons could benefit from additional ARIA attributes and keyboard handling.
<Button color="blue" type="submit" onClick={handleSaveToken} disabled={!!token} - ariaLabel="Save Token" + ariaLabel="Save access token" + role="button" + onKeyPress={(e) => e.key === 'Enter' && handleSaveToken(e)} > Save Token </Button> <Button type="button" onClick={handleClearToken} disabled={!token} - ariaLabel="Clear Token" + ariaLabel="Clear access token" + role="button" + onKeyPress={(e) => e.key === 'Enter' && handleClearToken(e)} > Clear Token </Button>widgets-samples/samples-meeting-app/webpack.config.js (1)
63-73: Remove commented code.The commented file-loader configuration should either be implemented or removed to maintain clean code.
- // { - // test: /\.(woff(2)?|ttf|eot|svg|png|gif)(\?v=\d+\.\d+\.\d+)?$/, - // use: [ - // { - // loader: 'file-loader', - // options: { - // outputPath: 'assets/', - // }, - // }, - // ], - // },widgets-samples/samples-meeting-app/src/WebexMeetingsWidgetDemo.jsx (3)
9-26: Consider extracting iOS detection logic.The iOS detection logic could be moved to a separate utility function for better maintainability and reusability.
+const isIOS151 = () => { + return typeof navigator !== 'undefined' && navigator.userAgent.includes('iPhone OS 15_1'); +}; + export default function WebexMeetingsWidgetDemo({token, fedramp}) { // ... other state declarations ... - const ON_IOS_15_1 = typeof navigator !== 'undefined' - && navigator.userAgent.includes('iPhone OS 15_1'); + const ON_IOS_15_1 = isIOS151();
60-72: Modernize fullscreen handling.The fullscreen implementation could be simplified and modernized:
const handleFullscreen = () => { const demoWidget = document.querySelector('.webex-meeting-widget-demo'); - - if (demoWidget.requestFullscreen) { - demoWidget.requestFullscreen(); - } else if (demoWidget.webkitRequestFullscreen) { - /* Safari */ - demoWidget.current.webkitRequestFullscreen(); - } else if (demoWidget.msRequestFullscreen) { - /* IE11 */ - demoWidget.msRequestFullscreen(); - } + if (!document.fullscreenElement) { + demoWidget.requestFullscreen?.() || + demoWidget.webkitRequestFullscreen?.() || + demoWidget.msRequestFullscreen?.(); + } };
141-151: Enhance error handling in getHydraId.The function could provide more detailed feedback about validation failures:
const getHydraId = (destination) => { const { type, id, cluster } = deconstructHydraId(destination); const UUID_REG = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; if (id && UUID_REG.test(id) && type === "ROOM") { return { room: true, destination: id, cluster }; } - return {}; + return { + room: false, + error: !id ? 'Invalid ID format' : + !UUID_REG.test(id) ? 'Invalid UUID format' : + type !== "ROOM" ? 'Invalid room type' : 'Unknown error' + }; };package.json (1)
10-10: Simplify workspace path pattern.The pattern
widgets-samples/**/**is redundant. A single**is sufficient for recursive matching.- "widgets-samples/**/**" + "widgets-samples/**/*"widgets-samples/samples-meeting-app/package.json (1)
18-52: Consider optimizing devDependencies.Some devDependencies might be redundant as they're already available in the root package.json:
- webpack and related packages
- babel packages
- style loaders
Consider moving common devDependencies to the root package.json and removing them from individual packages to reduce installation time and ensure version consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
widgets-samples/samples-meeting-app/src/webex-logo.pngis excluded by!**/*.pngyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (18)
.circleci/config.yml(1 hunks)copy_to_docs.sh(1 hunks)package.json(2 hunks)packages/@webex/widgets/package.json(1 hunks)widgets-samples/cc/samples-cc-react-app/package.json(1 hunks)widgets-samples/cc/samples-cc-react-app/tsconfig.json(1 hunks)widgets-samples/cc/samples-cc-react-app/webpack.config.js(1 hunks)widgets-samples/cc/samples-cc-wc-app/package.json(2 hunks)widgets-samples/cc/samples-cc-wc-app/tsconfig.json(1 hunks)widgets-samples/cc/samples-cc-wc-app/webpack.config.js(2 hunks)widgets-samples/index.html(2 hunks)widgets-samples/samples-meeting-app/package.json(1 hunks)widgets-samples/samples-meeting-app/src/App.jsx(1 hunks)widgets-samples/samples-meeting-app/src/App.scss(1 hunks)widgets-samples/samples-meeting-app/src/WebexMeetingsWidgetDemo.jsx(1 hunks)widgets-samples/samples-meeting-app/src/WebexMeetingsWidgetDemo.scss(1 hunks)widgets-samples/samples-meeting-app/src/index.html(1 hunks)widgets-samples/samples-meeting-app/webpack.config.js(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- widgets-samples/cc/samples-cc-react-app/tsconfig.json
- widgets-samples/samples-meeting-app/src/index.html
- widgets-samples/cc/samples-cc-react-app/package.json
- widgets-samples/cc/samples-cc-wc-app/tsconfig.json
- widgets-samples/samples-meeting-app/src/WebexMeetingsWidgetDemo.scss
- widgets-samples/samples-meeting-app/src/App.scss
🧰 Additional context used
🪛 Shellcheck (0.10.0)
copy_to_docs.sh
[warning] 14-14: Quote this to prevent word splitting.
(SC2046)
🔇 Additional comments (10)
widgets-samples/cc/samples-cc-wc-app/webpack.config.js (2)
4-4: Verify the base config import path.The path change suggests directory restructuring. Ensure the new path correctly resolves to the base webpack config.
✅ Verification successful
Base config import path is correct
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the webpack.config.js exists at the new path test -f webpack.config.js && echo "Base config exists at repository root" || echo "Base config not found"Length of output: 112
Script:
#!/bin/bash # Find the location of the webpack config file fd -p "samples-cc-wc-app/webpack.config.js$"Length of output: 101
17-20: Review the development server configuration changes.The changes include:
- Serving static files from the root directory instead of
public- Disabling auto-browser opening
- Using port 4000 (avoiding conflict with React app on 3000)
Verify that static assets are properly organized in the root directory since we're no longer using the
publicdirectory.✅ Verification successful
Static assets are properly organized, configuration changes are valid
The static assets are correctly organized within their respective module directories, with the main application files (index.html, app.js) located in the same directory as the webpack configuration. The development server configuration change to serve from the root directory is appropriate for this setup.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for static assets in the root directory echo "Static assets in root directory:" fd -t f -e html -e css -e js -e png -e ico -d 1Length of output: 142
Script:
#!/bin/bash # Check for public directory and static assets in project echo "Checking project structure for static assets:" fd -t f -e html -e css -e js -e png -e ico echo -e "\nChecking webpack configuration:" cat webpack.config.jsLength of output: 7943
widgets-samples/cc/samples-cc-react-app/webpack.config.js (1)
5-5: LGTM! Base config path updated consistently.The import path change aligns with the directory restructuring seen in the web components app config.
widgets-samples/samples-meeting-app/webpack.config.js (1)
14-18: Uncomment and verify the docs output path.The commented path for docs output suggests an incomplete implementation. Consider uncommenting and verifying the path if documentation hosting is required.
✅ Verification successful
Keep using the 'dist' directory configuration.
The commented docs path should remain commented out. Documentation building is handled by the root-level workspace build process through
samples:buildscript andcopy_to_docs.sh, not by individual sample app webpack configurations.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if docs directory exists and is used test -d docs && echo "Docs directory exists" || echo "Docs directory not found"Length of output: 73
Script:
#!/bin/bash # Check for README files and build documentation echo "=== README files ===" fd -t f "README.md" --exec head -n 20 {} echo -e "\n=== Build related files ===" fd -t f "(webpack|build|deploy)" --exec echo {} echo -e "\n=== Checking for docs in package.json ===" fd "package.json" --exec cat {} | jq -r 'select(.scripts != null) | .scripts | to_entries | .[] | select(.key | contains("doc") or contains("build") or contains("deploy")) | "\(.key): \(.value)"'Length of output: 3656
widgets-samples/samples-meeting-app/src/WebexMeetingsWidgetDemo.jsx (1)
1-8: LGTM! Well-structured imports.The component uses appropriate dependencies and follows React best practices.
widgets-samples/cc/samples-cc-wc-app/package.json (1)
2-2: LGTM! Package configuration aligns with project structure.The changes to project name and serve command appropriately reflect the new unified structure using Webpack dev server.
Also applies to: 17-17
package.json (1)
44-47: LGTM! Build and test scripts properly handle new sample apps.The scripts have been correctly updated to:
- Exclude sample apps from main test/build
- Include all sample apps in samples:build
- Run copy_to_docs.sh after building samples
packages/@webex/widgets/package.json (1)
10-12: LGTM! Clean and build scripts follow best practices.The new scripts properly handle cleaning before building, which helps prevent stale artifacts.
widgets-samples/samples-meeting-app/package.json (1)
14-15: Verify React version consistency.The same React version (18.3.1) is specified here. This should be verified and aligned with the core package.
.circleci/config.yml (1)
64-64: LGTM! Build command properly aligned with new script name.The command has been correctly updated to use
build:srcinstead ofbuild, matching the changes in package.json.
rarajes2
force-pushed
the
unify-samples
branch
from
c7c2faa to
fa3d314
Compare
rarajes2
force-pushed
the
unify-samples
branch
from
0103ff1 to
de99547
Compare
|
🎉 This PR is included in version 1.28.0-ccwidgets.12 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.28.0-ccconnectors.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This PR unifies all the widget samples app on one page.
JIRA - SPARK-569245
The following has been done as part of this PR:
widgets-samplesdirectory and inside that we haveccdirectory containing thesamples-cc-react-appandsamples-cc-wc-appsamples-meeting-appappbuild:srccommanddocsdirectory to be hosted on the github pagesamples:buildcommand to create the build and copy the build todocsdirectory.samples-cc-wc-appand updated theservecommand to utilize the webpack http dev serverVidcast - https://app.vidcast.io/share/3a91518d-e682-4838-b621-f5c6666c0bc3
Summary by CodeRabbit
New Features
Chores
Documentation