-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Migrate router and <app-view> to React #4525
Merged
Merged
Changes from 34 commits
Commits
Show all changes
37 commits
Select commit
Hold shift + click to select a range
ee529fe
Migrate router and <app-view> to React: skeleton
kravets-levko 94e3de1
Update layout on route change
kravets-levko 0e85dd8
Start moving page routes from angular to react
kravets-levko ce64f0d
Move page routes to react except of public dashboard and visualizatio…
kravets-levko 4058093
Move public dashboard and visualization embed routes to React
kravets-levko b4ff41c
Replace $route/$routeParams usages
kravets-levko d874669
Some cleanup
kravets-levko 8bb6c6d
Replace AngularJS $location service with implementation based on hist…
kravets-levko e4350f0
Minor fix to how ApplicationView handles route change
kravets-levko 933b50a
Explicitly use global layout for each page instead of handling relate…
kravets-levko d2e0f98
Error handling
kravets-levko 824ef59
Merge branch 'master' into migrate-router-to-react
kravets-levko b2b504e
Remove AngularJS and related dependencies
kravets-levko 9815996
Move Parameter factory method to a separate file
gabrieldutra 7306d21
Fix CSS (replace custom components with classes)
kravets-levko b140b55
Fix: keep other url parts when updating location partially; refine code
kravets-levko e075d40
Merge branch 'master' into migrate-router-to-react
kravets-levko 4f299fe
Fix tests
kravets-levko 0336dad
Make router work in multi-org mode (respect <base> tag)
kravets-levko 79111f2
Optimzation: don't resolve route if path didn't change
kravets-levko 5c94b70
Fix search input in header; error handling improvement (handle more e…
kravets-levko ebaf1e2
Fix page keys; fix navigateTo calls (third parameter not available)
kravets-levko 09c096c
Use relative links
kravets-levko 5b7d56c
Router: ignore location REPLACE events, resolve only on PUSH/POP
kravets-levko f77f602
Fix tests
kravets-levko efeecb6
Remove unused jQuery reference
kravets-levko 6e6c1df
Merge branch 'master' into migrate-router-to-react
kravets-levko d2e0beb
Show error from backend when creating Destination
gabrieldutra d4a769b
Remove route.resolve where not necessary (used constant values)
kravets-levko dea46de
New Query page: keep state on saving, reload when creating another ne…
kravets-levko dc4051a
Use currentRoute.key instead of hard-coded keys for page components
kravets-levko a54d917
Tidy up Router
kravets-levko d822d39
Tidy up location service
kravets-levko a92d1fa
Fix tests
kravets-levko 2755e12
Merge branch 'master' into migrate-router-to-react
kravets-levko f8e7334
Don't add parameters changes to browser's history
kravets-levko 7e3f9cf
Fix test (improved fix)
kravets-levko File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
61 changes: 61 additions & 0 deletions
61
client/app/components/ApplicationArea/AuthenticatedPageWrapper.jsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
import React, { useEffect, useState } from "react"; | ||
import PropTypes from "prop-types"; | ||
import ErrorBoundary from "@/components/ErrorBoundary"; | ||
import { Auth } from "@/services/auth"; | ||
import organizationStatus from "@/services/organizationStatus"; | ||
import ApplicationHeader from "./ApplicationHeader"; | ||
import ErrorMessage from "./ErrorMessage"; | ||
|
||
export default function AuthenticatedPageWrapper({ bodyClass, children }) { | ||
const [isAuthenticated, setIsAuthenticated] = useState(!!Auth.isAuthenticated()); | ||
|
||
useEffect(() => { | ||
let isCancelled = false; | ||
Promise.all([Auth.requireSession(), organizationStatus.refresh()]) | ||
.then(() => { | ||
if (!isCancelled) { | ||
setIsAuthenticated(!!Auth.isAuthenticated()); | ||
} | ||
}) | ||
.catch(() => { | ||
if (!isCancelled) { | ||
setIsAuthenticated(false); | ||
} | ||
}); | ||
return () => { | ||
isCancelled = true; | ||
}; | ||
}, []); | ||
|
||
useEffect(() => { | ||
if (bodyClass) { | ||
document.body.classList.toggle(bodyClass, true); | ||
return () => { | ||
document.body.classList.toggle(bodyClass, false); | ||
}; | ||
} | ||
}, [bodyClass]); | ||
|
||
if (!isAuthenticated) { | ||
return null; | ||
} | ||
|
||
return ( | ||
<> | ||
<ApplicationHeader /> | ||
<ErrorBoundary renderError={error => <ErrorMessage error={error} showOriginalMessage={false} />}> | ||
{children} | ||
</ErrorBoundary> | ||
</> | ||
); | ||
} | ||
|
||
AuthenticatedPageWrapper.propTypes = { | ||
bodyClass: PropTypes.string, | ||
children: PropTypes.node, | ||
}; | ||
|
||
AuthenticatedPageWrapper.defaultProps = { | ||
bodyClass: null, | ||
children: null, | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
import React from "react"; | ||
import PropTypes from "prop-types"; | ||
|
||
export default function ErrorMessage({ error, showOriginalMessage }) { | ||
if (!error) { | ||
return null; | ||
} | ||
|
||
console.error(error); | ||
|
||
const message = showOriginalMessage | ||
? error.message | ||
: "It seems like we encountered an error. Try refreshing this page or contact your administrator."; | ||
|
||
return ( | ||
<div className="fixed-container" data-test="ErrorMessage"> | ||
<div className="container"> | ||
<div className="col-md-8 col-md-push-2"> | ||
<div className="error-state bg-white tiled"> | ||
<div className="error-state__icon"> | ||
<i className="zmdi zmdi-alert-circle-o" /> | ||
</div> | ||
<div className="error-state__details"> | ||
<h4>{message}</h4> | ||
</div> | ||
</div> | ||
</div> | ||
</div> | ||
</div> | ||
); | ||
} | ||
|
||
ErrorMessage.propTypes = { | ||
error: PropTypes.object.isRequired, | ||
showOriginalMessage: PropTypes.bool, | ||
}; | ||
|
||
ErrorMessage.defaultProps = { | ||
showOriginalMessage: true, | ||
}; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Not blocking; just to understand if we can improve this further on)
Do we really need to set this class on the
body
element instead of one of the children that we can control w/ React?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ATM - yes, because it stretches few wrapper components when that class is set. To avoid this, we have to revisit some layout styles which I didn't want to do in this PR.