-
Notifications
You must be signed in to change notification settings - Fork 1
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
WEB: Update changes in websiite #296
Conversation
…webinar/registration
Reviewer's Guide by SourceryThis pull request updates various components and configurations across the Kloudlite application. Key changes include updating dependencies, modifying UI components, refactoring code, and adjusting content in multiple files. File-Level Changes
Tips
|
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.
Hey @nxtCoder19 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 2 issues found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
@@ -64,12 +63,25 @@ import { handleError } from '~/root/lib/utils/common'; | |||
// return <ResourceExtraAction options={options} />; | |||
// }; | |||
|
|||
const AppSelectItem = ({ label, value }: { label: string; value: string }) => { | |||
const AppSelectItem = ({ |
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.
issue (complexity): Consider simplifying the AppSelectItem component and reintroducing the Select component for image selection.
The changes introduce unnecessary complexity in the AppSelectItem component and remove useful functionality in the AppGeneral component. Here are suggestions to simplify the code while maintaining the intended functionality:
- Simplify AppSelectItem:
const AppSelectItem = ({
label,
value,
registry,
repository,
}: {
label: string;
value: string;
registry?: string;
repository?: string;
}) => {
const registryInfo = registry && repository ? `${registry}/${repository}` : value;
return (
<div>
<div className="flex flex-col">
<div>{label}</div>
<div className="bodySm text-text-soft">{registryInfo}</div>
</div>
</div>
);
};
This simplification removes the conditional rendering and always displays either the registry/repository information or the value, making the component more straightforward.
- Reintroduce the Select component for image selection in AppGeneral:
<Select
label="Select Images"
size="lg"
value={values.imageUrl}
placeholder="Select an image"
creatable
options={async () => imageList}
onChange={({ value }) => {
handleChange('imageUrl')(dummyEvent(value));
}}
showclear
noOptionMessage={
<div className="p-2xl bodyMd text-center">
Search for image or enter image name
</div>
}
error={!!errors.imageUrl}
message={errors.imageUrl}
loading={imageLoaded}
createLabel="Select"
/>
This change brings back the Select component, which provides better UX for image selection while maintaining the ability to enter custom image names.
These modifications simplify the code, improve maintainability, and preserve the intended functionality without unnecessary complexity.
|
||
|
||
|
||
export const WebinarUI = ({ userDetails, meetingStatus }: { userDetails: any, meetingStatus: string }) => { |
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.
issue (complexity): Consider simplifying the component structure and removing unused code.
The current implementation introduces unnecessary complexity. Consider the following improvements:
- Remove unused imports and commented-out code:
"use client";
import { Button } from 'kl-design-system/atoms/button';
import { TextInput } from 'kl-design-system/atoms/input';
import Popup from 'kl-design-system/molecule/popup';
import { cn } from 'kl-design-system/utils';
import { useState } from 'react';
import Container from '../components/container';
import { JoinWebinar } from '../components/join-webinar';
// Rest of the code...
- Simplify the HandleRegisterForm component:
const HandleRegisterForm = ({ visible, setVisible }: { visible: boolean, setVisible: (v: boolean) => void }) => {
const handleSubmit = (e: React.FormEvent) => {
e.preventDefault();
// Add form submission logic here
setVisible(false);
};
return (
<Popup.Form onSubmit={handleSubmit}>
<Popup.Content>
<div className="flex flex-col gap-2xl">
<TextInput
label="Full name"
size="lg"
placeholder="Enter your full name"
// Add necessary props like value and onChange
/>
{/* Add other form fields as needed */}
</div>
</Popup.Content>
<Popup.Footer>
<Popup.Button closable content="Cancel" variant="basic" onClick={() => setVisible(false)} />
<Popup.Button
type="submit"
content="Register"
variant="primary"
/>
</Popup.Footer>
</Popup.Form>
);
};
- If you need placeholders for future functionality, consider using TODO comments instead of large blocks of commented-out code. For example:
// TODO: Add validation schema
// TODO: Implement form submission logic
These changes will significantly reduce the complexity of the code while maintaining its core functionality. Remember to implement only the features that are currently needed, and use version control and documentation for future plans rather than keeping large blocks of commented-out code.
…webinar/registration
Summary by Sourcery
Update the website with new webinar UI components, enhance existing components with additional data handling, and improve documentation by adding onboarding content and removing outdated AI & ML workflows information.
New Features:
Enhancements:
Documentation:
Chores: