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

Migrate router and <app-view> to React #4525

Merged
merged 37 commits into from
Jan 20, 2020
Merged
Show file tree
Hide file tree
Changes from 36 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 Jan 7, 2020
94e3de1
Update layout on route change
kravets-levko Jan 7, 2020
0e85dd8
Start moving page routes from angular to react
kravets-levko Jan 8, 2020
ce64f0d
Move page routes to react except of public dashboard and visualizatio…
kravets-levko Jan 8, 2020
4058093
Move public dashboard and visualization embed routes to React
kravets-levko Jan 8, 2020
b4ff41c
Replace $route/$routeParams usages
kravets-levko Jan 8, 2020
d874669
Some cleanup
kravets-levko Jan 8, 2020
8bb6c6d
Replace AngularJS $location service with implementation based on hist…
kravets-levko Jan 10, 2020
e4350f0
Minor fix to how ApplicationView handles route change
kravets-levko Jan 10, 2020
933b50a
Explicitly use global layout for each page instead of handling relate…
kravets-levko Jan 10, 2020
d2e0f98
Error handling
kravets-levko Jan 10, 2020
824ef59
Merge branch 'master' into migrate-router-to-react
kravets-levko Jan 13, 2020
b2b504e
Remove AngularJS and related dependencies
kravets-levko Jan 13, 2020
9815996
Move Parameter factory method to a separate file
gabrieldutra Jan 13, 2020
7306d21
Fix CSS (replace custom components with classes)
kravets-levko Jan 14, 2020
b140b55
Fix: keep other url parts when updating location partially; refine code
kravets-levko Jan 14, 2020
e075d40
Merge branch 'master' into migrate-router-to-react
kravets-levko Jan 14, 2020
4f299fe
Fix tests
kravets-levko Jan 14, 2020
0336dad
Make router work in multi-org mode (respect <base> tag)
kravets-levko Jan 14, 2020
79111f2
Optimzation: don't resolve route if path didn't change
kravets-levko Jan 14, 2020
5c94b70
Fix search input in header; error handling improvement (handle more e…
kravets-levko Jan 16, 2020
ebaf1e2
Fix page keys; fix navigateTo calls (third parameter not available)
kravets-levko Jan 16, 2020
09c096c
Use relative links
kravets-levko Jan 16, 2020
5b7d56c
Router: ignore location REPLACE events, resolve only on PUSH/POP
kravets-levko Jan 16, 2020
f77f602
Fix tests
kravets-levko Jan 16, 2020
efeecb6
Remove unused jQuery reference
kravets-levko Jan 16, 2020
6e6c1df
Merge branch 'master' into migrate-router-to-react
kravets-levko Jan 16, 2020
d2e0beb
Show error from backend when creating Destination
gabrieldutra Jan 16, 2020
d4a769b
Remove route.resolve where not necessary (used constant values)
kravets-levko Jan 19, 2020
dea46de
New Query page: keep state on saving, reload when creating another ne…
kravets-levko Jan 19, 2020
dc4051a
Use currentRoute.key instead of hard-coded keys for page components
kravets-levko Jan 19, 2020
a54d917
Tidy up Router
kravets-levko Jan 19, 2020
d822d39
Tidy up location service
kravets-levko Jan 19, 2020
a92d1fa
Fix tests
kravets-levko Jan 19, 2020
2755e12
Merge branch 'master' into migrate-router-to-react
kravets-levko Jan 20, 2020
f8e7334
Don't add parameters changes to browser's history
kravets-levko Jan 20, 2020
7e3f9cf
Fix test (improved fix)
kravets-levko Jan 20, 2020
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
1 change: 0 additions & 1 deletion client/.babelrc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
"@babel/preset-react"
],
"plugins": [
"angularjs-annotate",
"@babel/plugin-proposal-class-properties",
"@babel/plugin-transform-object-assign",
["babel-plugin-transform-builtin-extend", {
Expand Down
35 changes: 16 additions & 19 deletions client/app/assets/less/inc/base.less
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ body {
font-family: @redash-font;
position: relative;

app-view {
#application-root {
padding-bottom: 15px;
}

&.headless {
app-view {
#application-root {
padding-top: 10px;
padding-bottom: 0;
}
Expand All @@ -45,11 +45,11 @@ body {
}
}

app-view {
#application-root {
min-height: 100vh;
}

app-view,
#application-root,
#app-content {
display: flex;
flex-direction: column;
Expand Down Expand Up @@ -93,11 +93,10 @@ strong {
@media (min-width: 768px) {
.settings-screen,
.home-page,
page-dashboard-list,
page-queries-list,
page-alerts-list,
alert-page,
queries-search-results-page,
.page-dashboard-list,
.page-queries-list,
.page-alerts-list,
.alert-page,
.fixed-container {
.container {
width: 750px;
Expand All @@ -108,11 +107,10 @@ strong {
@media (min-width: 992px) {
.settings-screen,
.home-page,
page-dashboard-list,
page-queries-list,
page-alerts-list,
alert-page,
queries-search-results-page,
.page-dashboard-list,
.page-queries-list,
.page-alerts-list,
.alert-page,
.fixed-container {
.container {
width: 970px;
Expand All @@ -123,11 +121,10 @@ strong {
@media (min-width: 1200px) {
.settings-screen,
.home-page,
page-dashboard-list,
page-queries-list,
page-alerts-list,
alert-page,
queries-search-results-page,
.page-dashboard-list,
.page-queries-list,
.page-alerts-list,
.alert-page,
.fixed-container {
.container {
width: 1170px;
Expand Down
6 changes: 3 additions & 3 deletions client/app/assets/less/redash/loading-indicator.less
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@
}
}

// hide indicator when app-view has content
app-view:not(:empty) ~ .loading-indicator {
// hide indicator when application has content
#application-root:not(:empty) ~ .loading-indicator {
opacity: 0;
transform: scale(0.9);
pointer-events: none;

* {
animation: none !important;
}
}
}
6 changes: 1 addition & 5 deletions client/app/assets/less/redash/query.less
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ body.fixed-layout {
padding: 0;
overflow: hidden;

app-view {
#application-root {
display: flex;
flex-direction: column;
padding-bottom: 0;
Expand Down Expand Up @@ -692,9 +692,5 @@ nav .rg-bottom {
h3 {
font-size: 18px;
}

favorites-control {
margin-top: -3px;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint-disable no-template-curly-in-string */

import React, { useRef } from "react";
import React, { useCallback, useRef } from "react";

import Dropdown from "antd/lib/dropdown";
import Button from "antd/lib/button";
Expand All @@ -9,25 +9,28 @@ import Menu from "antd/lib/menu";
import Input from "antd/lib/input";
import Tooltip from "antd/lib/tooltip";

import FavoritesDropdown from "./components/FavoritesDropdown";
import HelpTrigger from "@/components/HelpTrigger";
import CreateDashboardDialog from "@/components/dashboards/CreateDashboardDialog";
import navigateTo from "@/components/ApplicationArea/navigateTo";

import { currentUser, Auth, clientConfig } from "@/services/auth";
import { $location, $route } from "@/services/ng";
import { Dashboard } from "@/services/dashboard";
import { Query } from "@/services/query";
import frontendVersion from "@/version.json";
import logoUrl from "@/assets/images/redash_icon_small.png";

import "./AppHeader.less";
import FavoritesDropdown from "./FavoritesDropdown";
import "./index.less";

function onSearch(q) {
$location.path("/queries").search({ q });
$route.reload();
navigateTo(`queries?q=${encodeURIComponent(q)}`);
}

function DesktopNavbar() {
const showCreateDashboardDialog = useCallback(() => {
CreateDashboardDialog.showModal().result.catch(() => {}); // ignore dismiss
}, []);

return (
<div className="app-header" data-platform="desktop">
<div>
Expand Down Expand Up @@ -62,7 +65,7 @@ function DesktopNavbar() {
)}
{currentUser.hasPermission("create_dashboard") && (
<Menu.Item key="new-dashboard">
<a onMouseUp={() => CreateDashboardDialog.showModal()}>New Dashboard</a>
<a onMouseUp={showCreateDashboardDialog}>New Dashboard</a>
</Menu.Item>
)}
{currentUser.hasPermission("list_alerts") && (
Expand Down Expand Up @@ -249,7 +252,7 @@ function MobileNavbar() {
);
}

export default function AppHeader() {
export default function ApplicationHeader() {
return (
<nav className="app-header-wrapper">
<DesktopNavbar />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ nav .app-header {
justify-content: space-between;
margin-bottom: 10px;
background: white;
box-shadow: 0 4px 9px -3px rgba(102, 136, 153, .15);
box-shadow: 0 4px 9px -3px rgba(102, 136, 153, 0.15);

.darker {
color: #333 !important;

&:hover {
color: #2196F3 !important;
color: #2196f3 !important;
}
}

Expand All @@ -39,7 +39,7 @@ nav .app-header {
height: 50px;
border-bottom: 0;
}

.ant-btn {
font-weight: 500;

Expand Down Expand Up @@ -70,7 +70,7 @@ nav .app-header {
top: 2px;

svg {
transition: transform .2s cubic-bezier(.75,0,.25,1);
transition: transform 0.2s cubic-bezier(0.75, 0, 0.25, 1);
}
}

Expand Down Expand Up @@ -140,7 +140,7 @@ nav .app-header {
.menu-item-button {
padding: 0 10px;
}

.ant-menu-root {
margin: 0 5px;
}
Expand Down Expand Up @@ -198,10 +198,10 @@ nav .app-header {

.ant-dropdown-menu-item .help-trigger {
display: inline;
color: #2196F3;
color: #2196f3;
vertical-align: bottom;
}

.ant-dropdown-menu.favorites-dropdown {
margin-left: -10px;
}
}
61 changes: 61 additions & 0 deletions client/app/components/ApplicationArea/AuthenticatedPageWrapper.jsx
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);
Copy link
Member

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?

Copy link
Collaborator Author

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.

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,
};
40 changes: 40 additions & 0 deletions client/app/components/ApplicationArea/ErrorMessage.jsx
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,
};
Loading