Skip to content
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

Feature: Button that allows the user to switch the side where the sidebar is displayed #340

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 30 additions & 27 deletions src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { AboutPage, TimeTableSelectorPage, FaqsPage, NotFoundPage } from './page
import { getPath, config, dev_config, plausible } from './utils'
import Layout from './components/layout'
import * as Sentry from "@sentry/react";
import { SidebarProvider } from "./components/layout/SidebarPosition";

const configToUse = Number(import.meta.env.VITE_APP_PROD) ? config : dev_config

Expand Down Expand Up @@ -60,33 +61,35 @@ const App = () => {
const SentryRoutes = Sentry.withSentryReactRouterV6Routing(Routes);

return (
<BrowserRouter>
<CombinedProvider>
<SentryRoutes>
{pages.map((page, pageIdx) => (
<Route
path={page.path}
key={`page-${pageIdx}`}
element={
<Layout location={page.location} title={page.location} liquid={page.liquid}>
<div>
<page.element />
<Toaster/>
</div>
</Layout>
}
/>
))}
{redirects.map((redirect, redirectIdx) => (
<Route
path={redirect.from}
key={`redirect-${redirectIdx}`}
element={<Navigate replace to={redirect.to} />}
/>
))}
</SentryRoutes>
</CombinedProvider>
</BrowserRouter>
<SidebarProvider>
Copy link
Member

@tomaspalma tomaspalma Nov 14, 2024

Choose a reason for hiding this comment

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

In the same light as the previous comment, since the sidebar button to change sides should not be global, it does not really make sense to render the SidebarProvider here.

It should be inside the TimeTableScheduler.tsx page

<BrowserRouter>
<CombinedProvider>
<SentryRoutes>
{pages.map((page, pageIdx) => (
<Route
path={page.path}
key={`page-${pageIdx}`}
element={
<Layout location={page.location} title={page.location} liquid={page.liquid}>
<div>
<page.element />
<Toaster/>
</div>
</Layout>
}
/>
))}
{redirects.map((redirect, redirectIdx) => (
<Route
path={redirect.from}
key={`redirect-${redirectIdx}`}
element={<Navigate replace to={redirect.to} />}
/>
))}
</SentryRoutes>
</CombinedProvider>
</BrowserRouter>
</SidebarProvider>
)
}

Expand Down
18 changes: 16 additions & 2 deletions src/components/layout/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import {
AtSymbolIcon,
RectangleStackIcon,
QuestionMarkCircleIcon,
ArrowsRightLeftIcon,
} from '@heroicons/react/24/outline'
import { LogoNIAEFEUPImage } from '../../images'
import { getPath, config } from '../../utils'
import { useSidebarContext } from './SidebarPosition'

const navigation = [
{
Expand All @@ -34,6 +36,8 @@ type Props = {
}

const Header = ({ siteTitle, location }: Props) => {
const { toggleSidebarPosition } = useSidebarContext();

return (
<Disclosure
as="nav"
Expand Down Expand Up @@ -80,10 +84,20 @@ const Header = ({ siteTitle, location }: Props) => {
))}
</div>

<div className="hidden self-center md:inline-flex">
<div className='flex items-center space-x-2 relative top-1'>
<button
onClick={toggleSidebarPosition}
className="hidden md:inline-flex items-center justify-center w-8 h-8 bg-primary text-white rounded hover:text-opacity-75"
>
<ArrowsRightLeftIcon className='h-4 w-4' />
</button>

Comment on lines +88 to +94
Copy link
Member

Choose a reason for hiding this comment

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

I don't think the button to switch sides should be here on the navbar because not all pages have the sidebar and it looks weird for the about page and the FAQ page to have such button.

Can you move it to some place inside the sidebar? For example, near the download button or on the bottom of the sidebar.


<div className="hidden self-center md:inline-flex">

<DarkModeSwitch />

<DarkModeSwitch />
</div>
</div>
</div>
</div>
Expand Down
31 changes: 31 additions & 0 deletions src/components/layout/SidebarPosition.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { createCheckboxScope } from "@radix-ui/react-checkbox";
import { Children, createContext, useContext, useState } from "react";

type SidebarContextType = {
sidebarPosition: 'left' | 'right';
toggleSidebarPosition: () => void;
};

const SidebarContext = createContext<SidebarContextType | undefined>(undefined);

export const SidebarProvider: React.FC<{ children: React.ReactNode }> = ({ children }) => {
const [sidebarPosition, setSidebarPosition] = useState<'left' | 'right'>('right');

const toggleSidebarPosition = () => {
setSidebarPosition((prev) => (prev === 'right' ? 'left' : 'right'));
};

return (
<SidebarContext.Provider value={{ sidebarPosition, toggleSidebarPosition }}>
{children}
</SidebarContext.Provider>
);
};

export const useSidebarContext = () => {
const context = useContext(SidebarContext);
if (!context) {
throw new Error('useSidebarContext must be used within a SidebarProvider')
}
return context
}
36 changes: 26 additions & 10 deletions src/pages/TimeTableSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ import { Schedule, Sidebar } from '../components/planner'
import { CourseInfo, Major } from '../@types'
import MajorContext from '../contexts/MajorContext'
import CourseContext from '../contexts/CourseContext'
import { useSidebarContext } from '../components/layout/SidebarPosition'

const TimeTableSelectorPage = () => {
const [majors, setMajors] = useState<Major[]>([])
const [coursesInfo, setCoursesInfo] = useState([]);
const [pickedCourses, setPickedCourses] = useState<CourseInfo[]>(StorageAPI.getPickedCoursesStorage());
const [checkboxedCourses, setCheckboxedCourses] = useState<CourseInfo[]>(StorageAPI.getPickedCoursesStorage());
const [ucsModalOpen, setUcsModalOpen] = useState<boolean>(false);
const { sidebarPosition } = useSidebarContext();

//TODO: Looks suspicious
const [choosingNewCourse, setChoosingNewCourse] = useState<boolean>(false);
Expand All @@ -21,6 +23,8 @@ const TimeTableSelectorPage = () => {
setMajors(majors)
})
}, [])


return (
<MajorContext.Provider value={{ majors, setMajors }}>
<CourseContext.Provider value={
Expand All @@ -33,18 +37,30 @@ const TimeTableSelectorPage = () => {
}
}>
<div className="grid w-full grid-cols-12 gap-x-4 gap-y-4 px-4 py-4">
{/* Schedule Preview */}
<div className="lg:min-h-adjusted order-1 col-span-12 min-h-min rounded-md bg-lightest px-3 py-3 dark:bg-dark lg:col-span-9 2xl:px-5 2xl:py-5">
<div className="h-full w-full">
{sidebarPosition === 'left' ?(
<>
<div className='col-span-12 lg:col-span-3 min-h'>
<Sidebar />
</div>
<div className='col-span-12 lg:col-span-9 min-h rounded-md bg-lightest px-3 py-3 dark:bg-dark 2xl:px-5 2xl:py-5'>
<Schedule />
</div>
</>
) : (
<>
<div className='col-span-12 lg:col-span-9 min-h rounded-md bg-lightest px-3 py-3 dark:bg-dark 2xl:px-5 2xl:py-5'>
<Schedule />
</div>
</div>

<Sidebar />
</div>
<div className='col-span-12 lg:col-span-3 min-h'>
<Sidebar />
</div>
</>
)}
</div>
</CourseContext.Provider>
</MajorContext.Provider>
)
}
);
};


export default TimeTableSelectorPage;
export default TimeTableSelectorPage;