UI/webinar: Webinar page using dyte implementation #285
Conversation
nxtCoder19
requested review from
karthik1729 and
abdheshnayak
as code owners
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
Reviewer's Guide by SourceryThis pull request introduces a new webinar page using the Dyte SDK, along with several supporting components and configurations. Key changes include the addition of a pnpm lock file, ESLint configuration, and various React components for the webinar app. The implementation also involves setting up Tailwind CSS for styling and configuring Next.js for environment management. File-Level Changes
Tips
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @nxtCoder19 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟡 Security: 1 issue found
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟡 Documentation: 2 issues found
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| import { Link } from '@remix-run/react'; | ||
| import { Button } from 'kl-design-system/atoms/button'; | ||
| import { BrandLogo } from 'kl-design-system/branding/brand-logo'; | ||
| // import { mainUrl } from '../consts'; |
There was a problem hiding this comment.
suggestion: Remove commented out code
Consider removing the commented out import statement. If it's not needed, it's better to delete it entirely rather than leaving it as a comment to keep the codebase clean.
| @@ -0,0 +1,36 @@ | |||
| This is a [Next.js](https://nextjs.org/) project bootstrapped with [`create-next-app`](https://github.com/vercel/next.js/tree/canary/packages/create-next-app). | |||
There was a problem hiding this comment.
suggestion (documentation): Consider adding a brief introduction about the webinar app.
While the general Next.js information is helpful, it would be beneficial to include a short description of the webinar app's purpose and functionality at the beginning of the README.
| This is a [Next.js](https://nextjs.org/) project bootstrapped with [`create-next-app`](https://github.com/vercel/next.js/tree/canary/packages/create-next-app). | |
| # Webinar App | |
| This is a webinar application built with Next.js, designed to facilitate online seminars and interactive presentations. It offers features for hosting, scheduling, and participating in webinars. | |
| This project is bootstrapped with [`create-next-app`](https://github.com/vercel/next.js/tree/canary/packages/create-next-app) using [Next.js](https://nextjs.org/). |
| @@ -0,0 +1,36 @@ | |||
| This is a [Next.js](https://nextjs.org/) project bootstrapped with [`create-next-app`](https://github.com/vercel/next.js/tree/canary/packages/create-next-app). | |||
|
|
|||
| ## Getting Started | |||
There was a problem hiding this comment.
suggestion (documentation): Consider adding webinar app-specific setup instructions.
If there are any specific configuration steps or requirements for the webinar app, it would be helpful to include them in the 'Getting Started' section.
## Getting Started
### Webinar App Setup
1. Configure environment variables:
- Copy `.env.example` to `.env.local`
- Set `WEBINAR_API_KEY` and other required variables
2. Install dependencies:
npm install
3. Run the development server:
| import { useEffect, useState } from 'react'; | ||
| import { MyMeetingUI } from '../../orgs/my-meeting-ui'; | ||
|
|
||
| export default function App() { |
There was a problem hiding this comment.
issue (complexity): Consider refactoring the code to improve modularity and separation of concerns.
While the current implementation works, we can improve its readability and maintainability without over-engineering. Here are some suggestions:
- Extract the API call into a separate function:
const joinMeeting = async (name, email, meetingId) => {
const token = btoa(`${process.env.NEXT_PUBLIC_DYTE_ORG_ID}:${process.env.NEXT_PUBLIC_DYTE_API_KEY}`);
try {
const { data: { success, data } } = await axios.post(
`https://api.dyte.io/v2/meetings/${meetingId}/participants`,
{
name,
picture: "https://i.imgur.com/test.jpg",
preset_name: "webinar_viewer",
custom_participant_id: email,
},
{
headers: {
Authorization: `Basic ${token}`,
},
}
);
if (!success) {
throw new Error('Failed to join meeting');
}
return data.token;
} catch (error) {
console.error('Error joining meeting:', error);
throw error;
}
};- Create a custom hook for Dyte client initialization:
const useDyteInitialization = (authToken) => {
const [meeting, initMeeting] = useDyteClient();
useEffect(() => {
if (authToken) {
initMeeting({
authToken: authToken,
defaults: {
audio: false,
video: false,
},
});
}
}, [authToken, initMeeting]);
return meeting;
};- Simplify the main component:
export default function App() {
const [authToken, setAuthToken] = useState('');
const params = useSearchParams();
const meeting = useDyteInitialization(authToken);
useEffect(() => {
const meetingId = params.get('meetingId');
const name = params.get('name');
const email = params.get('email');
if (meetingId && name && email) {
joinMeeting(name, email, meetingId)
.then(setAuthToken)
.catch(error => console.error('Failed to join meeting:', error));
}
}, [params]);
return (
<DyteProvider value={meeting}>
<MyMeetingUI />
</DyteProvider>
);
}These changes maintain the overall structure while improving readability and potential reusability. The API call logic is now in a separate function, making it easier to test and modify. The Dyte client initialization is in a custom hook, which could be reused if needed. The main component is now more focused on orchestrating the meeting join flow.
Remember to add appropriate error handling and consider adding comments to explain the flow if it's not immediately clear from the code structure.
nxtCoder19
force-pushed
the
ui/webinar
branch
from
dd39b18 to
c6e945a
Compare
nxtCoder19
force-pushed
the
ui/webinar
branch
from
c6e945a to
8ec31e5
Compare
nxtCoder19
force-pushed
the
ui/webinar
branch
from
8ec31e5 to
bbacb32
Compare
UI/webinar: Webinar page using dyte implementation
UI/webinar: Webinar page using dyte implementation
UI/webinar: Webinar page using dyte implementation
Summary by Sourcery
Implement a new webinar page using Dyte SDK, including components for joining webinars and displaying meeting UI. Add a footer component with social media links and navigation menu. Introduce a theme switcher component. Add build and CI configurations, including a pnpm-lock.yaml for dependencies and an ESLint configuration for code quality. Provide documentation for getting started and deploying the application.
New Features:
Enhancements:
Build:
CI:
Documentation: