-
Notifications
You must be signed in to change notification settings - Fork 375
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
1757 new routing #3494
1757 new routing #3494
Changes from 13 commits
f16b9df
bfbb88d
735741a
363aef1
db6ae8c
4f8e624
3f5994b
392d214
6851830
cfa920e
046f546
6955c5c
1a150e2
9b19cb7
b168ab7
444df22
994f4b1
158f9ba
66084ef
da13136
1e6fa60
8435d8a
6b8b521
533e6da
7301a84
230d8da
6515f2c
40f4351
5da6ad6
5ea5986
e77fa4b
aeb6e1d
1f3d1f7
7c89cde
d5fe95d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
// Route paths for the app. | ||
|
||
var CATALOG_ROUTE = "/catalog/"; | ||
var CATALOG_MEMBER_ROUTE = `${CATALOG_ROUTE}:catalogMemberId`; | ||
|
||
module.exports = { | ||
CATALOG_ROUTE, | ||
CATALOG_MEMBER_ROUTE | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,15 @@ | ||
import React from "react"; | ||
import createReactClass from "create-react-class"; | ||
import PropTypes from "prop-types"; | ||
import { withRouter } from "react-router-dom"; | ||
import defined from "terriajs-cesium/Source/Core/defined"; | ||
import addedByUser from "../../Core/addedByUser"; | ||
import removeUserAddedData from "../../Models/removeUserAddedData"; | ||
import CatalogItem from "./CatalogItem"; | ||
import getAncestors from "../../Models/getAncestors"; | ||
import ObserveModelMixin from "../ObserveModelMixin"; | ||
import raiseErrorOnRejectedPromise from "../../Models/raiseErrorOnRejectedPromise"; | ||
import URI from "urijs"; | ||
|
||
const STATE_TO_TITLE = { | ||
loading: "Loading...", | ||
|
@@ -23,6 +25,7 @@ const DataCatalogItem = createReactClass({ | |
|
||
propTypes: { | ||
item: PropTypes.object.isRequired, | ||
match: PropTypes.object.isRequired, | ||
viewState: PropTypes.object.isRequired, | ||
removable: PropTypes.bool, | ||
terria: PropTypes.object | ||
|
@@ -73,9 +76,10 @@ const DataCatalogItem = createReactClass({ | |
}, | ||
|
||
isSelected() { | ||
return addedByUser(this.props.item) | ||
? this.props.viewState.userDataPreviewedItem === this.props.item | ||
: this.props.viewState.previewedItem === this.props.item; | ||
return ( | ||
this.props.item.uniqueId === | ||
URI.decode(this.props.match.params.catalogMemberId) | ||
); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe there's a good reason for this that isn't apparent to me, but it seems to me the UI should determine which catalog item is selected based on the When the UI derives its state from multiple (potentially conflicting) places (e.g. routing and the view state), it can quickly get confusing which one is authoritative. |
||
}, | ||
|
||
render() { | ||
|
@@ -84,6 +88,7 @@ const DataCatalogItem = createReactClass({ | |
<CatalogItem | ||
onTextClick={this.setPreviewedItem} | ||
selected={this.isSelected()} | ||
linkTo={URI.encode(item.uniqueId)} | ||
text={item.nameInCatalog} | ||
title={getAncestors(item) | ||
.map(member => member.nameInCatalog) | ||
|
@@ -114,4 +119,8 @@ const DataCatalogItem = createReactClass({ | |
} | ||
}); | ||
|
||
module.exports = DataCatalogItem; | ||
module.exports = { | ||
default: withRouter(DataCatalogItem), | ||
DataCatalogItem: withRouter(DataCatalogItem), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably not a huge problem, but if I'm not mistaken calling |
||
DataCatalogItemRaw: DataCatalogItem | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,7 @@ import React from "react"; | |
import createReactClass from "create-react-class"; | ||
import PropTypes from "prop-types"; | ||
import ObserveModelMixin from "../ObserveModelMixin"; | ||
import DataCatalogItem from "./DataCatalogItem.jsx"; | ||
import { DataCatalogItem } from "./DataCatalogItem.jsx"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure why you had to change this, since we're also exporting it as default. Maybe use ES exports for DataCatalogItem.jsx instead of mixing CJS and ES? |
||
import DataCatalogGroup from "./DataCatalogGroup.jsx"; | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
import React from "react"; | ||
import PropTypes from "prop-types"; | ||
import raiseErrorToUser from "../../Models/raiseErrorToUser"; | ||
|
||
// https://reactjs.org/docs/error-boundaries.html | ||
export default class ErrorBoundary extends React.Component { | ||
constructor(props) { | ||
super(props); | ||
this.state = { hasError: false }; | ||
} | ||
|
||
static getDerivedStateFromError(error) { | ||
// Update state so the next render will show the fallback UI. | ||
return { hasError: true }; | ||
} | ||
|
||
componentDidCatch(error, info) { | ||
const children = this.props.children; | ||
const name = | ||
(children && children.type && children.type.displayName) || | ||
children.type.name || | ||
"Unnamed Component"; | ||
const errorString = (error.toString && error.toString()) || error; | ||
|
||
this.props.terria.analytics.logEvent( | ||
"error", | ||
"boundary", | ||
name, | ||
errorString | ||
); | ||
raiseErrorToUser(this.props.terria, error); | ||
} | ||
|
||
render() { | ||
if (this.state.hasError) { | ||
// You can render any custom fallback UI | ||
return <h1>Something went wrong.</h1>; | ||
} | ||
|
||
return this.props.children; | ||
} | ||
} | ||
|
||
ErrorBoundary.propTypes = { | ||
terria: PropTypes.object.isRequired, | ||
children: PropTypes.node.isRequired | ||
}; |
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.
Why does this need a 300ms delay?