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

Add routing to dashboard and create a basic Auth page #2

Closed
wants to merge 7 commits into from

Conversation

st-jay
Copy link
Contributor

@st-jay st-jay commented Jul 10, 2022

Changes made

  • Added react-router-dom
  • Auth component served on <apiBasePath>/dashboard route
    • Clears the apiKey value from LocalStorage
    • Prompts the user for apiKey
    • Saves it in LocalStorage
    • Redirects to <apiBasePath>/dashboard/home
  • Added a route protection component which redirects user back to auth page if apiKey value is not present in localStorage
  • On 404, redirect the user to <apiBasePath>/dashboard/home

@st-jay st-jay self-assigned this Jul 10, 2022
@st-jay st-jay requested a review from nkshah2 July 10, 2022 08:04
Comment on lines 45 to 46
<h2>Log in to Dashboard</h2>
<button onClick={promptForApiKey}>Insert your API Key</button>
Copy link
Contributor

Choose a reason for hiding this comment

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

We dont need to have any user interaction here, on page open just show them an alert or something with an input in it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

const apiKey = prompt("Please enter your API key");
if (apiKey !== null && apiKey.length !== 0) {
setValueToStorage(apiKeyLocalStorageKey, apiKey);
setApiKey(apiKey);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not needed anymore, it was needed for redirection

Comment on lines 16 to 39
import React, { useEffect } from "react";
import { Navigate, Outlet } from "react-router-dom";

import { apiKeyLocalStorageKey } from "../../../constants";
import { getValueFromStorage } from "../../../storageService";

const ProtectedRoute = () => {
const [shouldRedirect, setShouldRedirect] = React.useState(false);

useEffect(() => {
const apiKey = getValueFromStorage(apiKeyLocalStorageKey);
if (apiKey === null || apiKey.length === 0) {
setShouldRedirect(true);
}
}, []);

if (shouldRedirect) {
return <><Navigate to="/" /></>;
}

return <Outlet />;
};

export default ProtectedRoute;
Copy link
Contributor

Choose a reason for hiding this comment

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

Just have an HOC that checks for the API key and renders a child if its found or redirects to "/" if it doesnt. Dont rely this heavily on the router

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated


// return to dashboard home once apiKey is stored in localStorage
if (apiKey.length > 0) {
return <Navigate to="/home" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Just use window API please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

src/index.tsx Outdated
root.render(
<React.StrictMode>
<App />
<BrowserRouter basename="/auth/dashboard">
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the variable here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@st-jay st-jay requested a review from nkshah2 July 10, 2022 09:05
@st-jay
Copy link
Contributor Author

st-jay commented Jul 19, 2022

Closing due to #3

@st-jay st-jay closed this Jul 19, 2022
@rishabhpoddar rishabhpoddar deleted the add-routing branch July 19, 2022 06:32
nkshah2 added a commit that referenced this pull request Sep 1, 2023
fix: dropdown flicker on user list #2
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.

2 participants