Skip to content

Conversation

@Anurag607
Copy link

I have made the main course page responsive, also separated all the styles files into a different directory for better organization.
Demo Video:
https://github.com/Samagra-Development/dsep-ui/assets/64116092/39947717-6cbe-4322-9523-170609a15f0a

</div>
</div>
<div className={`sidebar`}>
{/* {NavItemsData.map((el: any, i: number) => {
Copy link
Member

Choose a reason for hiding this comment

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

why have we commented these?

Copy link
Author

Choose a reason for hiding this comment

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

added updates as per the review, PTAL.

>
<Card className="p-2">
<Row>
<Card className="p-2" style={{ cursor: "pointer", position: "relative" }}>
Copy link
Member

Choose a reason for hiding this comment

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

can we move the contents of style={{}} in a dedicated css file?

Copy link
Member

Choose a reason for hiding this comment

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

Please make sure to do this for all inline style={{}} attributes

Copy link
Author

Choose a reason for hiding this comment

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

added updates as per the review, PTAL.

}}
>
{" "}
Svayam - NCERT
Copy link
Member

Choose a reason for hiding this comment

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

can we remove this hardcoding?

}}
>
{" "}
0 INR
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove this hard coding?

{/* <Nav.Link href="/my_courses" className="text-white"><FaGratipay /> My Courses</Nav.Link> */}

</Nav>
{/* <Nav className="me-auto">
Copy link
Member

Choose a reason for hiding this comment

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

delete comments if not required.

Copy link
Author

Choose a reason for hiding this comment

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

added updates as per the review, PTAL.

".sidebarel"
) as NodeListOf<HTMLDivElement>;

const closeSidebar = () => {
Copy link
Member

Choose a reason for hiding this comment

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

Can we move these to a dedicated .css file?

Copy link
Author

Choose a reason for hiding this comment

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

added updates as per the review, PTAL.

@techsavvyash techsavvyash changed the base branch from feat/minor-ui to c4gt-community August 2, 2023 04:06
src/App.tsx Outdated
<Router>
<div className="App" id={theme} style={{minHeight:'100vh'}}>
<div className="App" id={theme} style={{ minHeight: "100vh" }}>
{/* <button onClick={toggleTheme} className={`px-6 py-2.5 bg-sky-500 text-white font-medium text-xs leading-tight uppercase rounded-r-xl shadow-md mb-4 hover:bg-sky-700 hover:shadow-lg focus:bg-sky-700 focus:shadow-lg focus:outline-none focus:ring-0 active:bg-blue-800 active:shadow-lg transition duration-150 ease-in-out fixed bottom-0`}> {theme === "dark" ? <svg className="w-6 h-6" fill="none" stroke="currentColor" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"><path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d="M12 3v1m0 16v1m9-9h-1M4 12H3m15.364 6.364l-.707-.707M6.343 6.343l-.707-.707m12.728 0l-.707.707M6.343 17.657l-.707.707M16 12a4 4 0 11-8 0 4 4 0 018 0z" /></svg> : <svg className="w-6 h-6" fill="none" stroke="currentColor" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"><path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d="M20.354 15.354A9 9 0 018.646 3.646 9.003 9.003 0 0012 21a9.003 9.003 0 008.354-5.646z" /></svg>} </button> */}
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you remove this commented code?

Copy link
Author

Choose a reason for hiding this comment

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

added updates as per the review, PTAL.

course,
isMyCourse,
}) => {
console.log("mnop:", { course });
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove console

Copy link
Author

Choose a reason for hiding this comment

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

added updates as per the review, PTAL.

<div>
<FaCalendar />
<span>
{moment(course?.time?.range?.start).format("Do MMM, YYYY")}
Copy link
Collaborator

Choose a reason for hiding this comment

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

pass some fallback time to it

Copy link
Collaborator

Choose a reason for hiding this comment

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

do the same for all values

Copy link
Author

Choose a reason for hiding this comment

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

added updates as per the review, PTAL.

{cFilter.label}
</div> */}
<div className="filterEl">
{cFilter.component && (
Copy link
Collaborator

Choose a reason for hiding this comment

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

use proper naming conventions

Copy link
Author

Choose a reason for hiding this comment

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

added updates as per the review, PTAL.

@@ -0,0 +1,61 @@
const Menu = () => {
const menu = document.querySelector(".menu") as HTMLDivElement;
Copy link
Collaborator

Choose a reason for hiding this comment

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

why are we doing it like this ? can't we use some package or UI library for this ?

Copy link
Author

Choose a reason for hiding this comment

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

We could have avoided it, had the code been written using tailwind, in which case we could have used states and class names to achieve it. But bootstrap doesn't support this. Plus even we were to use a package we would still need to customize it via css to suit our needs.

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

3 participants