Add scheduled query browser#472
Add scheduled query browser#472n-riesco merged 34 commits intoplotly:masterfrom jakedex:scheduler-fe-state-1
Conversation
|
Thanks for the PR. I'll review it on Monday. @shannonlal I guess you may want to have a look at this PR, as this touches the UI. |
app/components/Layout.react.js
Outdated
| @@ -0,0 +1,31 @@ | |||
| import React from 'react'; | |||
| import PropTypes from 'prop-types'; | |||
|
|
|||
There was a problem hiding this comment.
Add comment with a description. Something like "this module contains a collection of helper components for layout purposes" would be enough, unless there is something about the usage of <Row> and <Column> that developers should know.
app/components/Modal.react.js
Outdated
| const Modal = props => { | ||
| const EnhancedClass = enhanceWithClickOutside(class extends Component { | ||
| handleClickOutside() { | ||
| // eslint-disable-next-line |
There was a problem hiding this comment.
remove // eslint-disable-next-line
app/components/Modal.react.js
Outdated
| }); | ||
|
|
||
| return props.open | ||
| ? ( |
There was a problem hiding this comment.
format ternary expressions as:
return props.open ? (
expressionIfTrue
) : (
expressionIfFalse
);
| } | ||
|
|
||
| /* TODO move this override */ | ||
| .scheduler .CodeMirror { |
There was a problem hiding this comment.
remove this change (not needed for this PR)
| import PropTypes from 'prop-types'; | ||
|
|
||
| import ReactDataGrid from 'react-data-grid'; | ||
| import ms from 'ms'; |
There was a problem hiding this comment.
add ms to devDependencies
Falcon uses webpack to bundle most of if its dependencies and for this reason most of them are listed as devDependencies.
Only dependencies that webpack is unable to bundle (e.g. node native modules) should be added as dependencies.
| import matchSorter from 'match-sorter'; | ||
| import Highlight from 'react-highlight'; | ||
|
|
||
| import { Link } from '../Link.react'; |
There was a problem hiding this comment.
use extensions, i.e. import { Link } from '../Link.react.js';
| import Highlight from 'react-highlight'; | ||
|
|
||
| import { Link } from '../Link.react'; | ||
| import { Row, Column } from '../Layout.react'; |
|
|
||
| import { Link } from '../Link.react'; | ||
| import { Row, Column } from '../Layout.react'; | ||
| import Modal from '../Modal.react'; |
| import { Link } from '../Link.react'; | ||
| import { Row, Column } from '../Layout.react'; | ||
| import Modal from '../Modal.react'; | ||
| import { plotlyUrl } from '../../utils/utils'; |
- Add comment - Remove old code - Add ms dev
|
|
||
| class IntervalFormatter extends React.Component { | ||
| static propTypes = { | ||
| value: PropTypes.object |
There was a problem hiding this comment.
similarly here: either rename value or add description.
| }; | ||
| const boxStyle = { boxSizing: 'border-box', width: '50%' }; | ||
|
|
||
| const MetaPreview = props => { |
There was a problem hiding this comment.
There is a styling issue with this component. See:
|
I'm not done writing the review but I've run out of time today. I'll continue tomorrow. Here are some general code style changes (for new code):
To discuss:
|
| <TabList> | ||
| <Tab>Connection</Tab> | ||
| <Tab disabled={queryPanelDisabled}>Query</Tab> | ||
| <Tab disabled={false}>Schedule</Tab> |
package.json
Outdated
| "dtrace-provider": "^0.8.6", | ||
| "font-awesome": "^4.6.1", | ||
| "ibm_db": "^2.3.0", | ||
| "match-sorter": "^2.2.3", |
package.json
Outdated
| "papaparse": "^4.3.7", | ||
| "pg": "^4.5.5", | ||
| "pg-hstore": "^2.3.2", | ||
| "react-click-outside": "^3.0.1", |
package.json
Outdated
| "pg": "^4.5.5", | ||
| "pg-hstore": "^2.3.2", | ||
| "react-click-outside": "^3.0.1", | ||
| "react-highlight": "^0.12.0", |
package.json
Outdated
| "sqlite3": "^3.1.13", | ||
| "tedious": "^2.1.4" | ||
| "tedious": "^2.1.4", | ||
| "tohash": "^1.0.2" |
| query: PropTypes.object | ||
| }; | ||
|
|
||
| export const MetaPreviewModal = props => ( |
There was a problem hiding this comment.
Is there any need for having two components: MetaPreview and MetaPreviewModal?
If possible, I'd like to have a single component with a name that doesn't require to document the purpose of these components (how about SchedulePreview?).
|
TODO
|
|
A few more styling issues (see screen capture below):
|
nicolaskruchten
changed the title
…/components/Settings/scheduler.test.jsx
|
@n-riesco we've made all the fixes as per your latest feedback |
|
I've tested it locally and it looks good. I'll merge it as soon as the tests in CircleCI succeed. @briandennis Thanks! It's been a pleasure to read such a clean PR. |
4 participants
Adds a tab to display the existing scheduled queries in a table. The queries can be searched and when clicked, display the full query string as well as a link to the grid.
Note, this first step only allows query viewing. Creation, deletion, and updating will come in later PRs.