From ed9d0446d5f44e58c02763b18cadb74169d929da Mon Sep 17 00:00:00 2001 From: Felix Edel Date: Tue, 23 Jun 2020 09:57:09 +0200 Subject: [PATCH] PF4: Update "fetching info ..." and refresh animation Currently the "refresh" animations look quite different depending on the page and the type of event (refresh button, initial page load, ...). This tries to make a start by updating the "Fetching info ..." animation (shown on initial page load) with Patternfly's "empty state" pattern [1] in combination with an animated spinner. For "Refreshable" pages, a similar animation is used in the upper right corner (like before) but with an updated spinner and icon. By using a dedicated React component rather than a base class, the "refresh" button can be more nicely integrated into the layout of the page/container and it doesn't look "out of scope" for the refreshable component. For the API page I've removed the refresh completely since it wasn't implemented at all. [1] https://www.patternfly.org/v4/documentation/react/components/emptystate Change-Id: I2274c212f14aece27ff49bfc7900d9b1a0fd01d0 --- web/src/App.jsx | 3 +- web/src/containers/Fetching.jsx | 74 ++++++++++++++++++++++++++++++ web/src/containers/Refreshable.jsx | 57 ----------------------- web/src/pages/Build.jsx | 26 ++++++++--- web/src/pages/BuildConsole.jsx | 27 ++++++++--- web/src/pages/BuildLogs.jsx | 26 ++++++++--- web/src/pages/Buildset.jsx | 27 ++++++++--- web/src/pages/ChangeStatus.jsx | 23 +++++++--- web/src/pages/Job.jsx | 17 ++++--- web/src/pages/Jobs.jsx | 26 ++++++++--- web/src/pages/Labels.jsx | 25 +++++++--- web/src/pages/LogFile.jsx | 18 +++++--- web/src/pages/Nodes.jsx | 25 +++++++--- web/src/pages/OpenApi.jsx | 6 +-- web/src/pages/Project.jsx | 23 +++++++--- web/src/pages/Projects.jsx | 31 +++++++++---- web/src/pages/Status.jsx | 23 +++++++--- web/src/pages/Tenants.jsx | 11 +++-- 18 files changed, 313 insertions(+), 155 deletions(-) create mode 100644 web/src/containers/Fetching.jsx delete mode 100644 web/src/containers/Refreshable.jsx diff --git a/web/src/App.jsx b/web/src/App.jsx index a811edfa82..9510a64bd6 100644 --- a/web/src/App.jsx +++ b/web/src/App.jsx @@ -64,6 +64,7 @@ import { } from '@patternfly/react-icons' import ErrorBoundary from './containers/ErrorBoundary' +import { Fetching } from './containers/Fetching' import SelectTz from './containers/timezone/SelectTz' import logo from './images/logo.svg' import { clearError } from './actions/errors' @@ -126,7 +127,7 @@ class App extends React.Component { const allRoutes = [] if (info.isFetching) { - return (

Fetching info...

) + return } this.menu // Do not include '/tenants' route in white-label setup diff --git a/web/src/containers/Fetching.jsx b/web/src/containers/Fetching.jsx new file mode 100644 index 0000000000..3abfe6949e --- /dev/null +++ b/web/src/containers/Fetching.jsx @@ -0,0 +1,74 @@ +// Copyright 2020 BMW Group +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. You may obtain +// a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +// License for the specific language governing permissions and limitations +// under the License. + +import * as React from 'react' +import PropTypes from 'prop-types' + +import { + Button, + Title, + EmptyState, + EmptyStateVariant, + Spinner, +} from '@patternfly/react-core' + +import { SyncIcon } from '@patternfly/react-icons' + +function Fetchable(props) { + const { isFetching, fetchCallback } = props + let content = null + + if (isFetching) { + content = ( +
+ +
+ ) + } else { + content = ( + + ) + } + + return ( +
+ {content} +
+ ) +} + +Fetchable.propTypes = { + isFetching: PropTypes.bool, + fetchCallback: PropTypes.func, +} + +function Fetching() { + return ( + + + + Fetching info... + + + ) +} + +export { Fetchable, Fetching } diff --git a/web/src/containers/Refreshable.jsx b/web/src/containers/Refreshable.jsx deleted file mode 100644 index c52f77b709..0000000000 --- a/web/src/containers/Refreshable.jsx +++ /dev/null @@ -1,57 +0,0 @@ -// Copyright 2018 Red Hat, Inc -// -// Licensed under the Apache License, Version 2.0 (the "License"); you may -// not use this file except in compliance with the License. You may obtain -// a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -// License for the specific language governing permissions and limitations -// under the License. - -// Boiler plate code to manage refresh button - -import * as React from 'react' -import PropTypes from 'prop-types' -import { - Icon, - Spinner -} from 'patternfly-react' - - -class Refreshable extends React.Component { - static propTypes = { - tenant: PropTypes.object, - remoteData: PropTypes.object, - } - - componentDidMount () { - if (this.props.tenant.name) { - this.updateData() - } - } - - componentDidUpdate (prevProps) { - if (this.props.tenant.name !== prevProps.tenant.name) { - this.updateData() - } - } - - renderSpinner () { - const { remoteData } = this.props - return ( - - {/* Lint warning jsx-a11y/anchor-is-valid */} - {/* eslint-disable-next-line */} - {this.updateData(true)}}> - refresh   - - - ) - } -} - -export default Refreshable diff --git a/web/src/pages/Build.jsx b/web/src/pages/Build.jsx index 2b2e7a5b4a..6d92aa1bbe 100644 --- a/web/src/pages/Build.jsx +++ b/web/src/pages/Build.jsx @@ -18,16 +18,17 @@ import PropTypes from 'prop-types' import { PageSection, PageSectionVariants } from '@patternfly/react-core' import { fetchBuildIfNeeded } from '../actions/build' -import Refreshable from '../containers/Refreshable' +import { Fetchable } from '../containers/Fetching' import Build from '../containers/build/Build' import Summary from '../containers/build/Summary' -class BuildPage extends Refreshable { +class BuildPage extends React.Component { static propTypes = { match: PropTypes.object.isRequired, remoteData: PropTypes.object, - tenant: PropTypes.object + tenant: PropTypes.object, + dispatch: PropTypes.func, } updateData = (force) => { @@ -37,7 +38,15 @@ class BuildPage extends Refreshable { componentDidMount () { document.title = 'Zuul Build' - super.componentDidMount() + if (this.props.tenant.name) { + this.updateData() + } + } + + componentDidUpdate (prevProps) { + if (this.props.tenant.name !== prevProps.tenant.name) { + this.updateData() + } } render () { @@ -45,9 +54,12 @@ class BuildPage extends Refreshable { const build = remoteData.builds[this.props.match.params.buildId] return ( -
- {this.renderSpinner()} -
+ + + {build && diff --git a/web/src/pages/BuildConsole.jsx b/web/src/pages/BuildConsole.jsx index 2646843e57..497608aa55 100644 --- a/web/src/pages/BuildConsole.jsx +++ b/web/src/pages/BuildConsole.jsx @@ -18,16 +18,18 @@ import PropTypes from 'prop-types' import { PageSection, PageSectionVariants } from '@patternfly/react-core' import { fetchBuildIfNeeded } from '../actions/build' -import Refreshable from '../containers/Refreshable' +import { Fetchable } from '../containers/Fetching' import Build from '../containers/build/Build' import Console from '../containers/build/Console' -class BuildConsolePage extends Refreshable { +class BuildConsolePage extends React.Component { static propTypes = { match: PropTypes.object.isRequired, remoteData: PropTypes.object, - tenant: PropTypes.object + tenant: PropTypes.object, + dispatch: PropTypes.func, + location: PropTypes.object, } updateData = (force) => { @@ -37,7 +39,15 @@ class BuildConsolePage extends Refreshable { componentDidMount () { document.title = 'Zuul Build' - super.componentDidMount() + if (this.props.tenant.name) { + this.updateData() + } + } + + componentDidUpdate (prevProps) { + if (this.props.tenant.name !== prevProps.tenant.name) { + this.updateData() + } } render () { @@ -47,9 +57,12 @@ class BuildConsolePage extends Refreshable { return ( -
- {this.renderSpinner()} -
+ + + {build && build.output && { @@ -37,7 +38,15 @@ class BuildLogsPage extends Refreshable { componentDidMount () { document.title = 'Zuul Build' - super.componentDidMount() + if (this.props.tenant.name) { + this.updateData() + } + } + + componentDidUpdate (prevProps) { + if (this.props.tenant.name !== prevProps.tenant.name) { + this.updateData() + } } render () { @@ -45,9 +54,12 @@ class BuildLogsPage extends Refreshable { const build = remoteData.builds[this.props.match.params.buildId] return ( -
- {this.renderSpinner()} -
+ + + {build && build.manifest && diff --git a/web/src/pages/Buildset.jsx b/web/src/pages/Buildset.jsx index 677083f069..3440cb326f 100644 --- a/web/src/pages/Buildset.jsx +++ b/web/src/pages/Buildset.jsx @@ -18,15 +18,16 @@ import PropTypes from 'prop-types' import { PageSection, PageSectionVariants } from '@patternfly/react-core' import { fetchBuildsetIfNeeded } from '../actions/build' -import Refreshable from '../containers/Refreshable' +import { Fetchable } from '../containers/Fetching' import Buildset from '../containers/build/Buildset' -class BuildsetPage extends Refreshable { +class BuildsetPage extends React.Component { static propTypes = { match: PropTypes.object.isRequired, remoteData: PropTypes.object, - tenant: PropTypes.object + tenant: PropTypes.object, + dispatch: PropTypes.func, } updateData = (force) => { @@ -36,17 +37,29 @@ class BuildsetPage extends Refreshable { componentDidMount () { document.title = 'Zuul Buildset' - super.componentDidMount() + if (this.props.tenant.name) { + this.updateData() + } + } + + componentDidUpdate (prevProps) { + if (this.props.tenant.name !== prevProps.tenant.name) { + this.updateData() + } } render () { const { remoteData } = this.props + const buildset = remoteData.buildsets[this.props.match.params.buildsetId] return ( -
- {this.renderSpinner()} -
+ + + {buildset && } ) diff --git a/web/src/pages/ChangeStatus.jsx b/web/src/pages/ChangeStatus.jsx index 0a7ec285ba..02846ef983 100644 --- a/web/src/pages/ChangeStatus.jsx +++ b/web/src/pages/ChangeStatus.jsx @@ -19,10 +19,10 @@ import { PageSection, PageSectionVariants } from '@patternfly/react-core' import { fetchChangeIfNeeded } from '../actions/change' import ChangePanel from '../containers/status/ChangePanel' -import Refreshable from '../containers/Refreshable' +import { Fetchable } from '../containers/Fetching' -class ChangeStatusPage extends Refreshable { +class ChangeStatusPage extends React.Component { static propTypes = { match: PropTypes.object.isRequired, tenant: PropTypes.object, @@ -43,7 +43,15 @@ class ChangeStatusPage extends Refreshable { componentDidMount () { document.title = this.props.match.params.changeId + ' | Zuul Status' - super.componentDidMount() + if (this.props.tenant.name) { + this.updateData() + } + } + + componentDidUpdate (prevProps) { + if (this.props.tenant.name !== prevProps.tenant.name) { + this.updateData() + } } componentWillUnmount () { @@ -58,9 +66,12 @@ class ChangeStatusPage extends Refreshable { const change = remoteData.change return ( -
- {this.renderSpinner()} -

+ + + {change && change.map((item, idx) => (
-
- {this.renderSpinner()} -
+ + + {tenantJobs && tenantJobs[jobName] && } ) diff --git a/web/src/pages/Jobs.jsx b/web/src/pages/Jobs.jsx index 84658025b0..4237a4ca06 100644 --- a/web/src/pages/Jobs.jsx +++ b/web/src/pages/Jobs.jsx @@ -18,34 +18,46 @@ import { connect } from 'react-redux' import { PageSection, PageSectionVariants } from '@patternfly/react-core' import { fetchJobsIfNeeded } from '../actions/jobs' -import Refreshable from '../containers/Refreshable' +import { Fetchable } from '../containers/Fetching' import Jobs from '../containers/jobs/Jobs' -class JobsPage extends Refreshable { +class JobsPage extends React.Component { static propTypes = { tenant: PropTypes.object, remoteData: PropTypes.object, dispatch: PropTypes.func } - updateData (force) { + updateData = (force) => { this.props.dispatch(fetchJobsIfNeeded(this.props.tenant, force)) } componentDidMount () { document.title = 'Zuul Jobs' - super.componentDidMount() + if (this.props.tenant.name) { + this.updateData() + } + } + + componentDidUpdate (prevProps) { + if (this.props.tenant.name !== prevProps.tenant.name) { + this.updateData() + } } render () { const { remoteData } = this.props + const jobs = remoteData.jobs[this.props.tenant.name] return ( -
- {this.renderSpinner()} -
+ + + {jobs && jobs.length > 0 && Loading...

) + return } const headerFormat = value => {value} @@ -61,9 +69,12 @@ class LabelsPage extends Refreshable { }) return ( -
- {this.renderSpinner()} -
+ + + { @@ -108,13 +113,14 @@ class LogFilePage extends Refreshable { render () { const { remoteData } = this.props + if (remoteData.isFetching) { + return + } + const build = this.props.build.builds[this.props.match.params.buildId] const severity = parse(this.props.location.search).severity return ( -
- {this.renderSpinner()} -
{remoteData.data && } ) diff --git a/web/src/pages/Nodes.jsx b/web/src/pages/Nodes.jsx index b4ce7ac79d..a7081ac594 100644 --- a/web/src/pages/Nodes.jsx +++ b/web/src/pages/Nodes.jsx @@ -20,23 +20,31 @@ import * as moment from 'moment' import { PageSection, PageSectionVariants } from '@patternfly/react-core' import { fetchNodesIfNeeded } from '../actions/nodes' -import Refreshable from '../containers/Refreshable' +import { Fetchable } from '../containers/Fetching' -class NodesPage extends Refreshable { +class NodesPage extends React.Component { static propTypes = { tenant: PropTypes.object, remoteData: PropTypes.object, dispatch: PropTypes.func } - updateData (force) { + updateData = (force) => { this.props.dispatch(fetchNodesIfNeeded(this.props.tenant, force)) } componentDidMount () { document.title = 'Zuul Nodes' - super.componentDidMount() + if (this.props.tenant.name) { + this.updateData() + } + } + + componentDidUpdate (prevProps) { + if (this.props.tenant.name !== prevProps.tenant.name) { + this.updateData() + } } render () { @@ -83,9 +91,12 @@ class NodesPage extends Refreshable { }) return ( -
- {this.renderSpinner()} -
+ + + -
- {this.renderSpinner()} -
) diff --git a/web/src/pages/Project.jsx b/web/src/pages/Project.jsx index 76558a14fa..8f76e9ff17 100644 --- a/web/src/pages/Project.jsx +++ b/web/src/pages/Project.jsx @@ -19,10 +19,10 @@ import { PageSection, PageSectionVariants } from '@patternfly/react-core' import Project from '../containers/project/Project' import { fetchProjectIfNeeded } from '../actions/project' -import Refreshable from '../containers/Refreshable' +import { Fetchable } from '../containers/Fetching' -class ProjectPage extends Refreshable { +class ProjectPage extends React.Component { static propTypes = { match: PropTypes.object.isRequired, tenant: PropTypes.object, @@ -37,7 +37,15 @@ class ProjectPage extends Refreshable { componentDidMount () { document.title = 'Zuul Project | ' + this.props.match.params.projectName - super.componentDidMount() + if (this.props.tenant.name) { + this.updateData() + } + } + + componentDidUpdate (prevProps) { + if (this.props.tenant.name !== prevProps.tenant.name) { + this.updateData() + } } render () { @@ -46,9 +54,12 @@ class ProjectPage extends Refreshable { const projectName = this.props.match.params.projectName return ( -
- {this.renderSpinner()} -
+ + + {tenantProjects && tenantProjects[projectName] && } diff --git a/web/src/pages/Projects.jsx b/web/src/pages/Projects.jsx index c183f7e3c4..13ee81bd8f 100644 --- a/web/src/pages/Projects.jsx +++ b/web/src/pages/Projects.jsx @@ -20,31 +20,43 @@ import { Table } from 'patternfly-react' import { PageSection, PageSectionVariants } from '@patternfly/react-core' import { fetchProjectsIfNeeded } from '../actions/projects' -import Refreshable from '../containers/Refreshable' +import { Fetchable, Fetching } from '../containers/Fetching' -class ProjectsPage extends Refreshable { +class ProjectsPage extends React.Component { static propTypes = { tenant: PropTypes.object, remoteData: PropTypes.object, dispatch: PropTypes.func } - updateData (force) { + updateData = (force) => { this.props.dispatch(fetchProjectsIfNeeded(this.props.tenant, force)) } componentDidMount () { document.title = 'Zuul Projects' - super.componentDidMount() + if (this.props.tenant.name) { + this.updateData() + } + } + + componentDidUpdate (prevProps) { + if (this.props.tenant.name !== prevProps.tenant.name) { + this.updateData() + } } render () { const { remoteData } = this.props const projects = remoteData.projects[this.props.tenant.name] + // TODO (felix): Can we somehow differentiate between "no projects yet" (due + // to fetching) and "no projects at all", so we could show an empty state + // in the latter case. The same applies for other pages like labels, nodes, + // buildsets, ... as well. if (!projects) { - return (

Loading...

) + return } const headerFormat = value => {value} @@ -86,9 +98,12 @@ class ProjectsPage extends Refreshable { }) return ( -
- {this.renderSpinner()} -
+ + + -
- {this.renderSpinner()} +
+ {this.setState({autoReload: e.target.checked})}} - style={{marginTop: '0px'}}> + style={{marginTop: '0px', marginLeft: '10px'}}> auto reload
diff --git a/web/src/pages/Tenants.jsx b/web/src/pages/Tenants.jsx index 3a811dbe5e..3b98d03edb 100644 --- a/web/src/pages/Tenants.jsx +++ b/web/src/pages/Tenants.jsx @@ -19,11 +19,11 @@ import { Link } from 'react-router-dom' import { Table } from 'patternfly-react' import { PageSection, PageSectionVariants } from '@patternfly/react-core' -import Refreshable from '../containers/Refreshable' +import { Fetching } from '../containers/Fetching' import { fetchTenantsIfNeeded } from '../actions/tenants' -class TenantsPage extends Refreshable { +class TenantsPage extends React.Component { static propTypes = { remoteData: PropTypes.object, dispatch: PropTypes.func @@ -43,6 +43,10 @@ class TenantsPage extends Refreshable { render () { const { remoteData } = this.props + if (remoteData.isFetching) { + return + } + const tenants = remoteData.tenants const headerFormat = value => {value} const cellFormat = (value) => ( @@ -79,9 +83,6 @@ class TenantsPage extends Refreshable { }) return ( -
- {this.renderSpinner()} -