Skip to content

Commit

Permalink
Update eslint config to include JSX files
Browse files Browse the repository at this point in the history
By default eslint only processes files with `.js` extension. Add
an override to include `.jsx`

Run `npm run lint:fix` to address to issues that can be automatically resolved.

Fix a11y bug with Task component keyboard nav that this highlighted.
  • Loading branch information
AlanGreene authored and tekton-robot committed Sep 29, 2023
1 parent b351651 commit 10474f4
Show file tree
Hide file tree
Showing 37 changed files with 471 additions and 343 deletions.
9 changes: 6 additions & 3 deletions .eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ limitations under the License.
module.exports = {
env: {
browser: true,
es2021: true,
es2022: true,
node: true
},
extends: [
Expand All @@ -31,6 +31,9 @@ module.exports = {
vi: true
},
overrides: [
{
files: ['*.js', '*.jsx']
},
{
files: ['*.cy.js'],
rules: {
Expand All @@ -42,7 +45,7 @@ module.exports = {
ecmaFeatures: {
jsx: true
},
ecmaVersion: 2021,
ecmaVersion: 2022,
sourceType: 'module'
},
plugins: ['notice', 'react', 'formatjs'],
Expand All @@ -53,7 +56,7 @@ module.exports = {
'formatjs/enforce-default-message': 'error',
'formatjs/enforce-id': 'error',
'formatjs/no-complex-selectors': 'error',
'formatjs/no-literal-string-in-jsx': 'error',
'formatjs/no-literal-string-in-jsx': 'off', // TODO: [AG] re-enable this after cleanup
'formatjs/no-multiple-whitespaces': 'error',
'formatjs/no-multiple-plurals': 'error',
'import/named': 'off',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class DetailsHeader extends Component {
stepStatus.terminated || {});
}

if (!startTime || !endTime || new Date(startTime).getTime() == 0) {
if (!startTime || !endTime || new Date(startTime).getTime() === 0) {
return null;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/components/Loading/Loading.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@ export default function Loading({ message }) {
<span className="tkn--loading-text">{messageToDisplay}</span>
</div>
);
};
}
24 changes: 16 additions & 8 deletions packages/components/src/components/Log/Log.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,9 @@ describe('Log', () => {
{ length: 19999 },
(_v, i) => `Line ${i + 1}\n`
).join('');
vi.spyOn(Element.prototype, 'getBoundingClientRect')
.mockReturnValue({ bottom: 0 });
vi.spyOn(Element.prototype, 'getBoundingClientRect').mockReturnValue({
bottom: 0
});
vi.spyOn(Element.prototype, 'scrollTop', 'get')
.mockReturnValue(-1) // to ensure el.scrollHeight - el.clientHeight > el.scrollTop i.e. 0 - 0 > -1
.mockReturnValueOnce(0);
Expand Down Expand Up @@ -154,8 +155,9 @@ describe('Log', () => {
{ length: 19999 },
(_v, i) => `Line ${i + 1}\n`
).join('');
vi.spyOn(Element.prototype, 'getBoundingClientRect')
.mockReturnValue({ bottom: 0 });
vi.spyOn(Element.prototype, 'getBoundingClientRect').mockReturnValue({
bottom: 0
});
vi.spyOn(Element.prototype, 'scrollTop', 'get')
.mockReturnValue(-1) // to ensure el.scrollHeight - el.clientHeight > el.scrollTop i.e. 0 - 0 > -1
.mockReturnValueOnce(0);
Expand Down Expand Up @@ -185,8 +187,11 @@ describe('Log', () => {
{ length: 20000 },
(_v, i) => `Line ${i + 1}\n`
).join('');
vi.spyOn(Element.prototype, 'getBoundingClientRect')
.mockReturnValue({ bottom: 0, top: 0, right: 0 });
vi.spyOn(Element.prototype, 'getBoundingClientRect').mockReturnValue({
bottom: 0,
top: 0,
right: 0
});
vi.spyOn(Element.prototype, 'scrollTop', 'get').mockReturnValue(1);
const spiedFn = vi.spyOn(Element.prototype, 'scrollTop', 'set'); // the scrollTop value is changed in scrollToBottomLog

Expand All @@ -211,8 +216,11 @@ describe('Log', () => {
{ length: 20000 },
(_v, i) => `Line ${i + 1}\n`
).join('');
vi.spyOn(Element.prototype, 'getBoundingClientRect')
.mockReturnValue({ bottom: 0, top: 0, right: 0 });
vi.spyOn(Element.prototype, 'getBoundingClientRect').mockReturnValue({
bottom: 0,
top: 0,
right: 0
});
vi.spyOn(Element.prototype, 'scrollTop', 'get').mockReturnValue(1);
vi.spyOn(Element.prototype, 'scrollHeight', 'get').mockReturnValue(2);
const spiedFn = vi.spyOn(Element.prototype, 'scrollTop', 'set'); // the scrollTop value is changed in scrollToBottomLog
Expand Down
62 changes: 37 additions & 25 deletions packages/components/src/components/PipelineRun/PipelineRun.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,16 @@ import StepDetails from '../StepDetails';
import TaskRunDetails from '../TaskRunDetails';
import TaskTree from '../TaskTree';

function getPipelineTask({ pipelineRun, selectedTaskId, taskRun }) {
const memberOf = taskRun?.metadata?.labels?.[labelConstants.MEMBER_OF];
const pipelineTask = (
pipelineRun.spec?.pipelineSpec?.[memberOf] ||
pipelineRun.status?.pipelineSpec?.[memberOf]
)?.find(task => task.name === selectedTaskId);

return pipelineTask;
}

export /* istanbul ignore next */ class PipelineRunContainer extends Component {
state = {
isLogsMaximized: false
Expand Down Expand Up @@ -112,16 +122,6 @@ export /* istanbul ignore next */ class PipelineRunContainer extends Component {
}));
};

getPipelineTask = ({ pipelineRun, selectedTaskId, taskRun }) => {
const memberOf = taskRun?.metadata?.labels?.[labelConstants.MEMBER_OF];
const pipelineTask = (
pipelineRun.spec?.pipelineSpec?.[memberOf] ||
pipelineRun.status?.pipelineSpec?.[memberOf]
)?.find(task => task.name === selectedTaskId);

return pipelineTask;
};

loadTaskRuns = () => {
const { pipelineRun } = this.props;
if (
Expand All @@ -131,28 +131,36 @@ export /* istanbul ignore next */ class PipelineRunContainer extends Component {
return [];
}

let { taskRuns } = this.props;
const { taskRuns } = this.props;
return taskRuns || [];
};

onTaskSelected = ({
selectedRetry: retry, selectedStepId, selectedTaskId, taskRunName
selectedRetry: retry,
selectedStepId,
selectedTaskId,
taskRunName
}) => {
const { handleTaskSelected, pipelineRun } = this.props;
const taskRuns = this.loadTaskRuns();
let taskRun = taskRuns.find(
({ metadata }) =>
metadata.labels?.[labelConstants.PIPELINE_TASK] === selectedTaskId
) || {};
const taskRun =
taskRuns.find(
({ metadata }) =>
metadata.labels?.[labelConstants.PIPELINE_TASK] === selectedTaskId
) || {};

const pipelineTask = this.getPipelineTask({ pipelineRun, selectedTaskId, taskRun });
const pipelineTask = getPipelineTask({
pipelineRun,
selectedTaskId,
taskRun
});
handleTaskSelected({
selectedRetry: retry,
selectedStepId,
selectedTaskId,
taskRunName: pipelineTask?.matrix ? taskRunName : undefined
});
}
};

render() {
const {
Expand Down Expand Up @@ -260,16 +268,20 @@ export /* istanbul ignore next */ class PipelineRunContainer extends Component {
}

const taskRuns = this.loadTaskRuns();
let taskRun = taskRuns.find(
({ metadata }) =>
metadata.labels?.[labelConstants.PIPELINE_TASK] === selectedTaskId
) || {};
let taskRun =
taskRuns.find(
({ metadata }) =>
metadata.labels?.[labelConstants.PIPELINE_TASK] === selectedTaskId
) || {};

const pipelineTask = this.getPipelineTask({ pipelineRun, selectedTaskId, taskRun });
const pipelineTask = getPipelineTask({
pipelineRun,
selectedTaskId,
taskRun
});
if (pipelineTask?.matrix && selectedTaskRunName) {
taskRun = taskRuns.find(
({ metadata }) =>
metadata.name === selectedTaskRunName
({ metadata }) => metadata.name === selectedTaskRunName
);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/components/src/components/Step/Step.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ class Step extends Component {
data-reason={reason}
data-selected={selected || undefined}
>
<span
<span // eslint-disable-line jsx-a11y/no-static-element-interactions
className="tkn--step-link"
tabIndex="0"
tabIndex="0" // eslint-disable-line jsx-a11y/no-noninteractive-tabindex
onClick={this.handleClick}
onKeyUp={e => e.key === 'Enter' && this.handleClick(e)}
>
Expand Down
81 changes: 50 additions & 31 deletions packages/components/src/components/Task/Task.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -137,18 +137,24 @@ class Task extends Component {
});

if (selectedRetry === '0') {
retryName = intl.formatMessage({
id: 'dashboard.pipelineRun.pipelineTaskName.firstAttempt',
defaultMessage: '{pipelineTaskName} (first attempt)'
}, { pipelineTaskName: displayName });
retryName = intl.formatMessage(
{
id: 'dashboard.pipelineRun.pipelineTaskName.firstAttempt',
defaultMessage: '{pipelineTaskName} (first attempt)'
},
{ pipelineTaskName: displayName }
);
} else {
retryName = intl.formatMessage({
id: 'dashboard.pipelineRun.pipelineTaskName.retry',
defaultMessage: '{pipelineTaskName} (retry {retryNumber, number})'
}, {
pipelineTaskName: displayName,
retryNumber: selectedRetry || taskRun.status.retriesStatus.length
});
retryName = intl.formatMessage(
{
id: 'dashboard.pipelineRun.pipelineTaskName.retry',
defaultMessage: '{pipelineTaskName} (retry {retryNumber, number})'
},
{
pipelineTaskName: displayName,
retryNumber: selectedRetry || taskRun.status.retriesStatus.length
}
);
}
}

Expand All @@ -161,8 +167,9 @@ class Task extends Component {
data-succeeded={succeeded}
data-selected={(expanded && !selectedStepId) || undefined}
>
<span
<span // eslint-disable-line jsx-a11y/no-static-element-interactions
className="tkn--task-link"
tabIndex={0} // eslint-disable-line jsx-a11y/no-noninteractive-tabindex
title={retryName || displayName}
onClick={this.handleTaskSelected}
onKeyUp={e => e.key === 'Enter' && this.handleTaskSelected(e)}
Expand All @@ -173,43 +180,55 @@ class Task extends Component {
reason={reason}
status={succeeded}
/>
<span className="tkn--task-link--name">{retryName || displayName}</span>
<span className="tkn--task-link--name">
{retryName || displayName}
</span>
{expanded && taskRun.status?.retriesStatus ? (
<OverflowMenu
ariaLabel={retryMenuTitle}
iconDescription={retryMenuTitle}
menuOptionsClass='tkn--task--retries-menu-options'
menuOptionsClass="tkn--task--retries-menu-options"
selectorPrimaryFocus="button:not([disabled])"
size="sm"
title={retryMenuTitle}
>
{
taskRun.status.retriesStatus.map((_retryStatus, index) => {
{taskRun.status.retriesStatus
.map((_retryStatus, index) => {
return {
id: index,
text: index === 0 ?
intl.formatMessage({
id: 'dashboard.pipelineRun.retries.viewFirstAttempt',
defaultMessage: 'View first attempt'
}) :
intl.formatMessage({
id: 'dashboard.pipelineRun.retries.viewRetry',
defaultMessage: 'View retry {retryNumber, number}'
}, { retryNumber: index })
text:
index === 0
? intl.formatMessage({
id: 'dashboard.pipelineRun.retries.viewFirstAttempt',
defaultMessage: 'View first attempt'
})
: intl.formatMessage(
{
id: 'dashboard.pipelineRun.retries.viewRetry',
defaultMessage: 'View retry {retryNumber, number}'
},
{ retryNumber: index }
)
};
}).concat([{ id: '', text: intl.formatMessage({
id: 'dashboard.pipelineRun.retries.viewLatestRetry',
defaultMessage: 'View latest retry'
})}]).map(item =>
})
.concat([
{
id: '',
text: intl.formatMessage({
id: 'dashboard.pipelineRun.retries.viewLatestRetry',
defaultMessage: 'View latest retry'
})
}
])
.map(item => (
<OverflowMenuItem
disabled={`${item.id}` === selectedRetry}
itemText={item.text}
key={item.text}
onClick={() => onRetryChange(item.id)}
requireTitle
/>
)
}
))}
</OverflowMenu>
) : null}
{expandIcon}
Expand Down
2 changes: 1 addition & 1 deletion packages/graph/src/components/Graph/Graph.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import ELK from 'elkjs/lib/elk.bundled';

import { ArrowRightMarker } from '@carbon/charts-react/diagrams/Marker';

import Edge from '../Edge/';
import Edge from '../Edge';
import Node from '../Node/Node';

function buildEdges({ direction, edges }) {
Expand Down
18 changes: 9 additions & 9 deletions src/containers/ClusterInterceptors/ClusterInterceptors.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ const clusterInterceptor = {

describe('ClusterInterceptors', () => {
it('renders loading state', async () => {
vi
.spyOn(API, 'useClusterInterceptors')
.mockImplementation(() => ({ isLoading: true }));
vi.spyOn(API, 'useClusterInterceptors').mockImplementation(() => ({
isLoading: true
}));
const { queryByText } = renderWithRouter(<ClusterInterceptorsContainer />, {
path: paths.clusterInterceptors.all(),
route: urls.clusterInterceptors.all()
Expand All @@ -41,9 +41,9 @@ describe('ClusterInterceptors', () => {
});

it('renders data', async () => {
vi
.spyOn(API, 'useClusterInterceptors')
.mockImplementation(() => ({ data: [clusterInterceptor] }));
vi.spyOn(API, 'useClusterInterceptors').mockImplementation(() => ({
data: [clusterInterceptor]
}));
const { queryByText } = renderWithRouter(<ClusterInterceptorsContainer />, {
path: paths.clusterInterceptors.all(),
route: urls.clusterInterceptors.all()
Expand All @@ -54,9 +54,9 @@ describe('ClusterInterceptors', () => {

it('handles error', async () => {
const error = 'fake_errorMessage';
vi
.spyOn(API, 'useClusterInterceptors')
.mockImplementation(() => ({ error }));
vi.spyOn(API, 'useClusterInterceptors').mockImplementation(() => ({
error
}));
const { queryByText } = renderWithRouter(<ClusterInterceptorsContainer />, {
path: paths.clusterInterceptors.all(),
route: urls.clusterInterceptors.all()
Expand Down
Loading

0 comments on commit 10474f4

Please sign in to comment.