Skip to content

fixed user + admin navbars #31

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fixed user + admin navbars #31

wants to merge 3 commits into from

Conversation

ysharma25
Copy link
Collaborator

Navbar for users and admin are now different

Copy link
Collaborator

@ranonl ranonl left a comment

Choose a reason for hiding this comment

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

Everything here technically works completely fine, but out of good coding practices, lets think about whether it's actually necessary/worth it to have these separate components for the user version of the NavBar. You can actually condense all the logic into the NavBar files that already existed in something like 5 lines of code. Why would we want to do this? Well, when other pages want to create an instance of the NavBar, we don't want them to have to worry about handling the logic for which to display. That can get very confusing and it's just kinda a breach of abstraction. If we condense all the logic into the NavBar components themselves, then whenever someone else calls NavBar, they don't have to worry and can trust that it will show either the user or admin view accordingly. Just to help out with how to do this, here's one way:

For both the MobileNavbar and AdminLoginNavbar files:

  • Create a state (search up "State Hooks" if you don't know how to use state, or ask me -> it's a very very important concept!) at the top of your NavBar component (just under where the function is defined) and let it represent whether we want to display the user or the admin view.
    - For example: const [isAdmin, setIsAdmin] = useState(false); is one way to do this

  • Create a new function within the component. This function should, based on the value of isAdmin, choose what to display on the NavBar. As we know, the only difference between the user and admin Navbars is whether it displays "View Appointments" or "Schedule Appointment." So, in this function, we will return one of two things:
    - 1) If isAdmin is true, we will return the Link to appointment view (line 12 in the AdminLoginNavbar)
    - 2) If isAdmin is false, we will return the Link to appointment scheduler (your line 12 in UserNavbar)

  • Finally, where the NavBar components (both normal and mobile) used to create the Link component that we now want to be conditionally rendered, replace it with {theFunctionYouMade}

I've attached one way it could look like. Please make sure you understand exactly what's going on. Make sure to make these changes for both the normal and mobile NavBars.

Screen Shot 2022-04-06 at 12 08 14 PM

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