diff options
| -rw-r--r-- | web/src/Misc.jsx | 7 | ||||
| -rw-r--r-- | web/src/containers/build/BuildTable.jsx | 125 | ||||
| -rw-r--r-- | web/src/containers/build/BuildsetTable.jsx | 92 | ||||
| -rw-r--r-- | web/src/containers/build/Misc.jsx | 44 | ||||
| -rw-r--r-- | web/src/index.css | 23 | ||||
| -rw-r--r-- | web/src/pages/Builds.jsx | 2 | ||||
| -rw-r--r-- | web/src/pages/Buildsets.jsx | 2 |
7 files changed, 143 insertions, 152 deletions
diff --git a/web/src/Misc.jsx b/web/src/Misc.jsx index ba2d62b02..e209ed042 100644 --- a/web/src/Misc.jsx +++ b/web/src/Misc.jsx @@ -92,4 +92,9 @@ function buildExternalTableLink(buildish) { return null } -export { removeHash, ExternalLink, buildExternalLink, buildExternalTableLink } +// https://github.com/kitze/conditional-wrap +// appears to be the first implementation of this pattern +const ConditionalWrapper = ({ condition, wrapper, children }) => + condition ? wrapper(children) : children + +export { removeHash, ExternalLink, buildExternalLink, buildExternalTableLink, ConditionalWrapper } diff --git a/web/src/containers/build/BuildTable.jsx b/web/src/containers/build/BuildTable.jsx index 00e6b3eba..c7cb15601 100644 --- a/web/src/containers/build/BuildTable.jsx +++ b/web/src/containers/build/BuildTable.jsx @@ -15,7 +15,6 @@ import React from 'react' import PropTypes from 'prop-types' import { connect } from 'react-redux' -import { Link } from 'react-router-dom' import { Button, EmptyState, @@ -47,8 +46,14 @@ import * as moment from 'moment' import { BuildResult, BuildResultWithIcon, IconProperty } from './Misc' import { buildExternalTableLink } from '../../Misc' -function BuildTable(props) { - const { builds, fetching, onClearFilters, tenant, timezone } = props +function BuildTable({ + builds, + fetching, + onClearFilters, + tenant, + timezone, + history, +}) { const columns = [ { title: <IconProperty icon={<BuildIcon />} value="Job" />, @@ -87,28 +92,23 @@ function BuildTable(props) { ] function createBuildRow(build) { - // This link will be defined on each cell of the current row as this is the - // only way to define a valid HTML link on a table row. Although we could - // simply define an onClick handler on the whole row and programatically - // switch to the buildresult page, this wouldn't provide the same - // look-and-feel as a plain HTML link. - const buildResultLink = ( - <Link - to={`${tenant.linkPrefix}/build/${build.uuid}`} - className="zuul-stretched-link" - /> - ) - const build_link = buildExternalTableLink(build) + const changeOrRefLink = buildExternalTableLink(build) return { + // Pass the build's uuid as row id, so we can use it later on in the + // action handler to build the link to the build result page for each row. + id: build.uuid, cells: [ { // To allow passing anything else than simple string values to a table // cell, we must use the title attribute. title: ( <> - {buildResultLink} - <BuildResultWithIcon result={build.result} colored={build.voting}> + <BuildResultWithIcon + result={build.result} + colored={build.voting} + link={`${tenant.linkPrefix}/build/${build.uuid}`} + > {build.job_name} {!build.voting && ' (non-voting)'} </BuildResultWithIcon> @@ -116,74 +116,36 @@ function BuildTable(props) { ), }, { - title: ( - <> - {buildResultLink} - <span>{build.project}</span> - </> - ), + title: build.project, }, { - title: ( - <> - {buildResultLink} - <span>{build.branch ? build.branch : build.ref}</span> - </> - ), + title: build.branch ? build.branch : build.ref, }, { - title: ( - <> - {buildResultLink} - <span>{build.pipeline}</span> - </> - ), + title: build.pipeline, }, { - title: ( - <> - {buildResultLink} - {build_link && ( - <span style={{ zIndex: 1, position: 'relative' }}> - {build_link} - </span> - )} - </> - ), + title: changeOrRefLink && changeOrRefLink, }, { - title: ( - <> - {buildResultLink} - <span> - {moment - .duration(build.duration, 'seconds') - .format('h [hr] m [min] s [sec]')} - </span> - </> - ), + title: moment + .duration(build.duration, 'seconds') + .format('h [hr] m [min] s [sec]'), }, { - title: ( - <> - {buildResultLink} - <span> - {build.start_time && - moment - .utc(build.start_time) - .tz(timezone) - .format('YYYY-MM-DD HH:mm:ss')} - </span> - </> - ), + title: moment + .utc(build.start_time) + .tz(timezone) + .format('YYYY-MM-DD HH:mm:ss'), }, { - title: ( - <> - {buildResultLink} - <BuildResult result={build.result} colored={build.voting} /> - </> - ), + title: ( + <BuildResult + result={build.result} + link={`${tenant.linkPrefix}/build/${build.uuid}`} + colored={build.voting} + /> + ), }, ], } @@ -209,6 +171,9 @@ function BuildTable(props) { } let rows = [] + // For the fetching row we don't need any actions, so we keep them empty by + // default. + let actions = [] if (fetching) { rows = createFetchingRow() // The dataLabel property is used to show the column header in a list-like @@ -219,6 +184,18 @@ function BuildTable(props) { columns[0].dataLabel = '' } else { rows = builds.map((build) => createBuildRow(build)) + // This list of actions will be applied to each row in the table. For + // row-specific actions we must evaluate the individual row data provided to + // the onClick handler. + actions = [ + { + title: 'Show build result', + onClick: (event, rowId, rowData) => + // The row's id contains the build's uuid, so we can use this to build + // the correct link. + history.push(`${tenant.linkPrefix}/build/${rowData.id}`), + }, + ] } return ( @@ -228,6 +205,7 @@ function BuildTable(props) { variant={TableVariant.compact} cells={columns} rows={rows} + actions={actions} className="zuul-build-table" > <TableHeader /> @@ -261,6 +239,7 @@ BuildTable.propTypes = { onClearFilters: PropTypes.func.isRequired, tenant: PropTypes.object.isRequired, timezone: PropTypes.string.isRequired, + history: PropTypes.object.isRequired, } export default connect((state) => ({ diff --git a/web/src/containers/build/BuildsetTable.jsx b/web/src/containers/build/BuildsetTable.jsx index dcd7efb0c..f0947971d 100644 --- a/web/src/containers/build/BuildsetTable.jsx +++ b/web/src/containers/build/BuildsetTable.jsx @@ -15,7 +15,6 @@ import React from 'react' import PropTypes from 'prop-types' import { connect } from 'react-redux' -import { Link } from 'react-router-dom' import { Button, EmptyState, @@ -43,8 +42,13 @@ import { import { BuildResult, BuildResultWithIcon, IconProperty } from './Misc' import { buildExternalTableLink } from '../../Misc' -function BuildsetTable(props) { - const { buildsets, fetching, onClearFilters, tenant } = props +function BuildsetTable({ + buildsets, + fetching, + onClearFilters, + tenant, + history, +}) { const columns = [ { title: <IconProperty icon={<CubeIcon />} value="Project" />, @@ -69,68 +73,41 @@ function BuildsetTable(props) { ] function createBuildsetRow(buildset) { - // This link will be defined on each cell of the current row as this is the - // only way to define a valid HTML link on a table row. Although we could - // simply define an onClick handler on the whole row and programatically - // switch to the buildresult page, this wouldn't provide the same - // look-and-feel as a plain HTML link. - const buildsetResultLink = ( - <Link - to={`${tenant.linkPrefix}/buildset/${buildset.uuid}`} - className="zuul-stretched-link" - /> - ) - const buildset_link = buildExternalTableLink(buildset) + const changeOrRefLink = buildExternalTableLink(buildset) return { + // Pass the buildset's uuid as row id, so we can use it later on in the + // action handler to build the link to the build result page for each row. + id: buildset.uuid, cells: [ { // To allow passing anything else than simple string values to a table // cell, we must use the title attribute. title: ( - <> - {buildsetResultLink} - <BuildResultWithIcon result={buildset.result}> - {buildset.project} - </BuildResultWithIcon> - </> + <BuildResultWithIcon + result={buildset.result} + link={`${tenant.linkPrefix}/buildset/${buildset.uuid}`}> + {buildset.project} + </BuildResultWithIcon> ), }, { - title: ( - <> - {buildsetResultLink} - <span>{buildset.branch ? buildset.branch : buildset.ref}</span> - </> - ), + title: buildset.branch ? buildset.branch : buildset.ref, }, { - title: ( - <> - {buildsetResultLink} - <span>{buildset.pipeline}</span> - </> - ), + title: buildset.pipeline, }, { - title: ( - <> - {buildsetResultLink} - {buildset_link && ( - <span style={{ zIndex: 1, position: 'relative' }}> - {buildset_link} - </span> - )} - </> - ), + title: changeOrRefLink && changeOrRefLink, }, { - title: ( - <> - {buildsetResultLink} - <BuildResult result={buildset.result} /> - </> - ), + title: ( + <BuildResult + result={buildset.result} + link={`${tenant.linkPrefix}/buildset/${buildset.uuid}`} + > + </BuildResult> + ), }, ], } @@ -156,6 +133,9 @@ function BuildsetTable(props) { } let rows = [] + // For the fetching row we don't need any actions, so we keep them empty by + // default. + let actions = [] if (fetching) { rows = createFetchingRow() // The dataLabel property is used to show the column header in a list-like @@ -166,6 +146,18 @@ function BuildsetTable(props) { columns[0].dataLabel = '' } else { rows = buildsets.map((buildset) => createBuildsetRow(buildset)) + // This list of actions will be applied to each row in the table. For + // row-specific actions we must evaluate the individual row data provided to + // the onClick handler. + actions = [ + { + title: 'Show buildset result', + onClick: (event, rowId, rowData) => + // The row's id contains the buildset's uuid, so we can use it to + // build the correct link. + history.push(`${tenant.linkPrefix}/buildset/${rowData.id}`), + }, + ] } return ( @@ -175,6 +167,7 @@ function BuildsetTable(props) { variant={TableVariant.compact} cells={columns} rows={rows} + actions={actions} className="zuul-build-table" > <TableHeader /> @@ -207,6 +200,7 @@ BuildsetTable.propTypes = { fetching: PropTypes.bool.isRequired, onClearFilters: PropTypes.func.isRequired, tenant: PropTypes.object.isRequired, + history: PropTypes.object.isRequired, } export default connect((state) => ({ tenant: state.tenant }))(BuildsetTable) diff --git a/web/src/containers/build/Misc.jsx b/web/src/containers/build/Misc.jsx index f77f7de7a..1d02992c0 100644 --- a/web/src/containers/build/Misc.jsx +++ b/web/src/containers/build/Misc.jsx @@ -14,13 +14,17 @@ import * as React from 'react' import PropTypes from 'prop-types' -import { Label } from '@patternfly/react-core' +import { Link } from 'react-router-dom' +import { + Label, +} from '@patternfly/react-core' import { CheckIcon, ExclamationIcon, QuestionIcon, TimesIcon, } from '@patternfly/react-icons' +import { ConditionalWrapper } from '../../Misc' const RESULT_ICON_CONFIGS = { SUCCESS: { @@ -82,15 +86,25 @@ const DEFAULT_RESULT_ICON_CONFIG = { } function BuildResult(props) { - const { result, colored = true } = props + const { result, link = undefined, colored = true } = props const iconConfig = RESULT_ICON_CONFIGS[result] || DEFAULT_RESULT_ICON_CONFIG const color = colored ? iconConfig.color : 'inherit' - return <span style={{ color: color }}>{result}</span> + return ( + <span style={{ color: color }}> + <ConditionalWrapper + condition={link} + wrapper={children => <Link to={link} style={{ color: color }}>{children}</Link>} + > + {result} + </ConditionalWrapper> + </span> + ) } BuildResult.propTypes = { result: PropTypes.string, + link: PropTypes.string, colored: PropTypes.bool, } @@ -117,7 +131,7 @@ BuildResultBadge.propTypes = { } function BuildResultWithIcon(props) { - const { result, colored = true, size = 'sm' } = props + const { result, link = undefined, colored = true, size = 'sm' } = props const iconConfig = RESULT_ICON_CONFIGS[result] || DEFAULT_RESULT_ICON_CONFIG // Define the verticalAlign based on the size @@ -132,20 +146,26 @@ function BuildResultWithIcon(props) { return ( <span style={{ color: color }}> - <Icon - size={size} - style={{ - marginRight: 'var(--pf-global--spacer--sm)', - verticalAlign: verticalAlign, - }} - /> - {props.children} + <ConditionalWrapper + condition={link} + wrapper={children => <Link to={link} style={{ color: color }}>{children}</Link>} + > + <Icon + size={size} + style={{ + marginRight: 'var(--pf-global--spacer--sm)', + verticalAlign: verticalAlign, + }} + /> + {props.children} + </ConditionalWrapper> </span> ) } BuildResultWithIcon.propTypes = { result: PropTypes.string, + link: PropTypes.string, colored: PropTypes.bool, size: PropTypes.string, children: PropTypes.node, diff --git a/web/src/index.css b/web/src/index.css index 83c73b613..2f6602c1c 100644 --- a/web/src/index.css +++ b/web/src/index.css @@ -51,10 +51,6 @@ a.refresh { font-weight: bold; } -.zuul-build-list:hover a { - text-decoration: none; -} - /* Keep the normal font-size for compact tables */ .zuul-build-table td { font-size: var(--pf-global--FontSize--md); @@ -98,20 +94,13 @@ a.refresh { .zuul-build-table tbody tr:hover td:first-child::before { background-color: var(--pf-global--active-color--100); } -} -/* Make a link stretch the whole parent element (in this case the whole table - cell) */ -.zuul-stretched-link::after { - position: absolute; - top: 0; - right: 0; - bottom: 0; - left: 0; - z-index: 0; - pointer-events: auto; - content: ""; - background-color: rgba(0,0,0,0); + /* Hide the action column with the build link on larger screen. This is only + needed for the mobile version as we can't use the "magnifying-glass icon + on hover" effect there. */ + .zuul-build-table .pf-c-table__action { + display: none; + } } /* diff --git a/web/src/pages/Builds.jsx b/web/src/pages/Builds.jsx index 144f9ab85..bb4e4cf47 100644 --- a/web/src/pages/Builds.jsx +++ b/web/src/pages/Builds.jsx @@ -151,6 +151,7 @@ class BuildsPage extends React.Component { } render() { + const { history } = this.props const { builds, fetching, filters } = this.state return ( <PageSection variant={PageSectionVariants.light}> @@ -163,6 +164,7 @@ class BuildsPage extends React.Component { builds={builds} fetching={fetching} onClearFilters={this.handleClearFilters} + history={history} /> </PageSection> ) diff --git a/web/src/pages/Buildsets.jsx b/web/src/pages/Buildsets.jsx index 602f4d0d8..a9372e8b0 100644 --- a/web/src/pages/Buildsets.jsx +++ b/web/src/pages/Buildsets.jsx @@ -139,6 +139,7 @@ class BuildsetsPage extends React.Component { } render() { + const { history } = this.props const { buildsets, fetching, filters } = this.state return ( <PageSection variant={PageSectionVariants.light}> @@ -151,6 +152,7 @@ class BuildsetsPage extends React.Component { buildsets={buildsets} fetching={fetching} onClearFilters={this.handleClearFilters} + history={history} /> </PageSection> ) |