ManPhamQuang / COSC2769-rmit

Further web programming - COSC2769
cosc-2769-rmit.vercel.app
MIT License
0 stars 0 forks source link

Implementation of Navbar on Expert Dashboard #62

Closed 8bitzz closed 3 years ago

8bitzz commented 3 years ago

Description

Discussion

  1. Shall we apply the same Navigation Bar as other pages on Expert Dashboard or creating another NavBar for this page?
  2. Currently, I am using AuthContext for checking the state of user and implement logic for Logout button. If I implement the logic this way, I might need to refractor some code in Expert Dashboard to look somehow like this
    
    // dashboard index.js
    import { AuthContext } from "../../context/authContext/AuthContext";
    import { logout } from "../../context/authContext/AuthActions";

export default function ExpertDashboard() {

const { state, dispatch } = useContext(AuthContext);

const handleLogout = (e) => { localStorage.removeItem("user"); localStorage.removeItem("accessToken"); dispatch(logout()); router.push("/login"); e.preventDefault(); };

useEffect(() => { // Navigate user to Login page if can not find token if (!state.token) { router.push("/login"); } }, []);

return (

{state.user && }

) }

ExpertDashboard.getLayout = (page) => {page};

 But I am not sure how to pass the `handleLogout` function to the `DashboardLayout` since it receives `children`, not `props`

```js
function DashboardLayout({ children }) {

}

Could you help to provide ideas for implementation @ManPhamQuang ? Thanks a bunch!

ManPhamQuang commented 3 years ago

@8bitzz I think you can move both the useEffect (the one that checks for token) and the log out function into the Dashboard Layout file. Since every dashboard screen already requires expert login.

ManPhamQuang commented 3 years ago

I think the navbar inside dashboard screens shouldn't have search bar and categories drop down. I think it only need the user dropdown and the logo icon for returning to home page. Since dashboard screen should be only for experts to manage their rooms and such, i think they wouldn't need categories dropdown or search bar. What do you think?

8bitzz commented 3 years ago

@ManPhamQuang Oke cool 👍 I will try implementing that way and create new PR so that you can review. Btw, I did not have much experiencing in Hamburger menu so any advices or tutorials would be much of help. Thanks again for your help!

ManPhamQuang commented 3 years ago

I followed this series for implementing the sidebar in expert dashboard. He's using React with Tailwind which is quite close to our project's setup. However ,if you find it too troublesome (as there are some quirks with next.js), feel free to implement all the logic functions in the navbar and I'll implement the responsive functionalities for you.

8bitzz commented 3 years ago

That's cool, I will take a look and let you know if it is too troublesome 👍 . Btw, NextJS complains state undefined when I implement AuthContext inside the DashboardLayout component.

function DashboardLayout({ children }) {
const { state, dispatch } = useContext(AuthContext);

useEffect(() => {
      // Navigate user to Login page if can not find token
      if (!state.token) {
        router.push("/login");
      }
  }, []);

}

It works if I put those code in the dashboard index.js file. Any ideas?

ManPhamQuang commented 3 years ago

It's due to the getLayout defined in _app.js is higher than the AuthContextProvider

// _app.js
function MyApp({ Component, pageProps }) {
  const getLayout = Component.getLayout || ((page) => page);
  return getLayout(
    <AuthContextProvider>
      <Component {...pageProps} />
    </AuthContextProvider>
  );
});

I think there should be a future PR where we further discuss the Layout component in _app.js and refactor code so that the navbar in user side and expert side is implemented in the same way. I notice that you're using next.js's dynamic import for the user's navbar whereas I use next.js's Layout for dashboard sidebar.

For now, you can simply add the AuthContextProvider in the expert dashboard page to get around this problem and keep working on your PR.

// dashboard/index.jsx
ExpertDashboard.getLayout = (page) => (
    <AuthContextProvider>
        <DashboardLayout>{page}</DashboardLayout>;
    </AuthContextProvider>
);
8bitzz commented 3 years ago

Please create another ticket with your suggestion for the implementation and tag me. Using Layout seems to be a better practice.