From b2d4e6345c757a6de39e8a3bc8a7683fe2f5a813 Mon Sep 17 00:00:00 2001 From: Brian Espinosa Date: Wed, 3 Mar 2021 15:51:51 -0800 Subject: [PATCH] chore(FDS-149): Resolve eslint `no-` prefixed errors and `multiline-comment-style` (#170) Fixes eslint rules: 'multiline-comment-style': 0, 'no-alert': 0, 'no-extra-boolean-cast': 0, 'no-prototype-builtins': 0, 'no-useless-return': 0, Co-authored-by: Manuel Ramirez --- .eslintrc.js | 5 - docs/src/components/PropTable/PropTable.js | 18 +-- .../src/modules/ActionButton/ActionButton.js | 18 +-- .../src/modules/ActionEdit/ActionEdit.js | 12 +- packages/cascara/src/shared/recordUtils.js | 5 +- packages/cascara/src/ui/Chat/Chat.js | 2 +- .../ui/Chat/fixtures/utils/currentPlatform.js | 2 +- .../ui/Dashboard/widgets/WidgetStatsStat.js | 4 +- .../cascara/src/ui/Pagination/Pagination.js | 38 +++--- .../cascara/src/ui/Table/ActionBar.test.js | 10 +- packages/cascara/src/ui/Table/Table.js | 30 ++--- packages/cascara/src/ui/Table/Table.test.js | 118 +++++++++--------- packages/cascara/src/ui/Table/TableRow.js | 2 +- .../fixtures/ActionStack.Table.fixture.js | 1 - .../fixtures/Conditional.Actions.fixture.js | 13 +- .../src/ui/Table/fixtures/EdiTable.fixture.js | 1 - .../cascara/src/ui/Table/fixtures/Export.js | 1 - 17 files changed, 136 insertions(+), 144 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index b8441a4ba..b788774c8 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -19,11 +19,6 @@ module.exports = { 'eslint-comments/no-unused-disable': 0, 'eslint-comments/require-description': 0, 'jest/no-done-callback': 0, - 'multiline-comment-style': 0, - 'no-alert': 0, - 'no-extra-boolean-cast': 0, - 'no-prototype-builtins': 0, - 'no-useless-return': 0, 'react/forbid-prop-types': 0, 'react/jsx-key': 0, 'react/jsx-no-bind': 0, diff --git a/docs/src/components/PropTable/PropTable.js b/docs/src/components/PropTable/PropTable.js index 949c2f885..ed053e436 100644 --- a/docs/src/components/PropTable/PropTable.js +++ b/docs/src/components/PropTable/PropTable.js @@ -49,15 +49,15 @@ const PropTable = ({ docData, ...rest }) => { {JSON.stringify(propData?.type?.value, null, ' ')} ) : ( - /* propData.type.value.map((value, i) => { - const item = value.name ? ( - {value.name} - ) : ( - {value.value} - ); - - return item; - }) */ + // propData.type.value.map((value, i) => { + // const item = value.name ? ( + // {value.name} + // ) : ( + // {value.value} + // ); + // + // return item; + // }) 
                         {JSON.stringify(propData?.type?.value, null, '  ')}
diff --git a/packages/cascara/src/modules/ActionButton/ActionButton.js b/packages/cascara/src/modules/ActionButton/ActionButton.js index 977b4254f..4bc706280 100644 --- a/packages/cascara/src/modules/ActionButton/ActionButton.js +++ b/packages/cascara/src/modules/ActionButton/ActionButton.js @@ -22,15 +22,15 @@ const ActionButton = ({ const { isEditing, onAction, record } = useContext(ModuleContext); const { content = label, ...restWithoutLabel } = rest; - /** - initially, this was called actionName, but now we ...spread - all props into the button. So the correct way of calling it - is just 'name'. - - This is a breaking change. in order to prevent any breakage - in the our Apps, we are temporarily deriving it from one if - the other is not passed. Once we have the resources to go - and update our Apps we will revisit. */ + // + // initially, this was called actionName, but now we ...spread + // all props into the button. So the correct way of calling it + // is just 'name'. + // + // This is a breaking change. in order to prevent any breakage + // in the our Apps, we are temporarily deriving it from one if + // the other is not passed. Once we have the resources to go + // and update our Apps we will revisit. const name = actionName || rest.name; // FDS-137: use action name for button name if no content is specified diff --git a/packages/cascara/src/modules/ActionEdit/ActionEdit.js b/packages/cascara/src/modules/ActionEdit/ActionEdit.js index 0b40c92df..2301b0678 100644 --- a/packages/cascara/src/modules/ActionEdit/ActionEdit.js +++ b/packages/cascara/src/modules/ActionEdit/ActionEdit.js @@ -32,9 +32,8 @@ const ActionEdit = ({ dataTestIDs, editLabel = 'Edit' }) => { const recordId = record[uniqueIdAttribute]; const whenAnotherRowIsEditing = Boolean(idOfRecordInEditMode); - /** - * this seems like ugly, we need to find a better way - * to ease testing.. */ + // this seems like ugly, we need to find a better way + // to ease testing.. const cancelTestId = {}; const editTestId = {}; const saveTestId = {}; @@ -61,15 +60,14 @@ const ActionEdit = ({ dataTestIDs, editLabel = 'Edit' }) => { const handleCancel = () => { isDirty - ? // eslint-disable-next-line no-restricted-globals + ? // eslint-disable-next-line no-restricted-globals, no-alert -- For now we do not have our own confirmation dialog so we are using native confirms confirm('Abandon unsaved changes?') && handleReset() : handleReset(); }; const handleEdit = () => { - /** - * FDS-91: We are resetting the form with whatever is in record. - * We don't know if this is the best way to do it in React. */ + // FDS-91: We are resetting the form with whatever is in record. + // We don't know if this is the best way to do it in React. reset({ ...record }); onAction( // fake target diff --git a/packages/cascara/src/shared/recordUtils.js b/packages/cascara/src/shared/recordUtils.js index 5786fbfa2..08ddb47f8 100644 --- a/packages/cascara/src/shared/recordUtils.js +++ b/packages/cascara/src/shared/recordUtils.js @@ -1,5 +1,8 @@ const getAttributeValueFromRecord = (attribute, record = {}) => { - const attributeExists = record.hasOwnProperty(attribute); + const attributeExists = Object.prototype.hasOwnProperty.call( + record, + attribute + ); if (attributeExists) { return record[attribute]; diff --git a/packages/cascara/src/ui/Chat/Chat.js b/packages/cascara/src/ui/Chat/Chat.js index c971688e2..8616f9219 100644 --- a/packages/cascara/src/ui/Chat/Chat.js +++ b/packages/cascara/src/ui/Chat/Chat.js @@ -84,7 +84,7 @@ const Chat = ({ sessionUserID, messages, users }) => { const isTranslated = Boolean(msg.isTranslated); // This can probably get cleaned up later. // Only return if we have a defined component for this type - return Boolean(getMessageObject) + return getMessageObject ? getMessageObject({ attached: getMessageGroup(msg, previousMessage, nextMessage), handleScrollToBottom, diff --git a/packages/cascara/src/ui/Chat/fixtures/utils/currentPlatform.js b/packages/cascara/src/ui/Chat/fixtures/utils/currentPlatform.js index e60bb8f63..58b3d8743 100644 --- a/packages/cascara/src/ui/Chat/fixtures/utils/currentPlatform.js +++ b/packages/cascara/src/ui/Chat/fixtures/utils/currentPlatform.js @@ -1,5 +1,5 @@ const platforms = { - cordova: Boolean(window.hasOwnProperty('cordova')), + cordova: Boolean(Object.prototype.hasOwnProperty.call(window, 'cordova')), electron: Boolean(process.versions['electron']), }; diff --git a/packages/cascara/src/ui/Dashboard/widgets/WidgetStatsStat.js b/packages/cascara/src/ui/Dashboard/widgets/WidgetStatsStat.js index 7ac39f4c7..39a7eea66 100644 --- a/packages/cascara/src/ui/Dashboard/widgets/WidgetStatsStat.js +++ b/packages/cascara/src/ui/Dashboard/widgets/WidgetStatsStat.js @@ -17,8 +17,8 @@ const WidgetStatsStat = ({ onClick, label, value, sub }) => { return ( { - /** - * Handles pagination changes - * - * This function updates the pagination values - * and invokes upstream logic to handle the change. - * - * @param {Event} _ Usually the click event - * @param {Object} component Component object passed by SUIR - * @param {String} component.name The name of the component - * @param {Any} component.value The new value in the component */ + // + // Handles pagination changes + // + // This function updates the pagination values + // and invokes upstream logic to handle the change. + // + // @param {Event} _ Usually the click event + // @param {Object} component Component object passed by SUIR + // @param {String} component.name The name of the component + // @param {Any} component.value The new value in the component const handlePaginationChange = (_, component) => { let newPage = currentPage; let newitemsPerPageLimit = itemsPerPageLimit; @@ -73,15 +73,15 @@ const Pagination = ({ }); }; - /** - * Handles button's click event - * - * This handler decides wether to increase or decrease page - * based on button name. It acts as a proxy for `handlePaginationChange` - * - * @param {Event} _ Usually the click event - * @param {Object} component Component object passed by SUIR - * @param {String} component.name The name of the component */ + // + // Handles button's click event + // + // This handler decides wether to increase or decrease page + // based on button name. It acts as a proxy for `handlePaginationChange` + // + // @param {Event} _ Usually the click event + // @param {Object} component Component object passed by SUIR + // @param {String} component.name The name of the component const handleButtonClick = (_, button) => { let newPage = currentPage; diff --git a/packages/cascara/src/ui/Table/ActionBar.test.js b/packages/cascara/src/ui/Table/ActionBar.test.js index 22a6af52e..cde52564e 100644 --- a/packages/cascara/src/ui/Table/ActionBar.test.js +++ b/packages/cascara/src/ui/Table/ActionBar.test.js @@ -9,8 +9,8 @@ const ACTION_MODULES = { }; describe('ActionBar', () => { - /** - * */ + // + // describe('component tree', () => { const actions = [ { @@ -33,9 +33,9 @@ describe('ActionBar', () => { const { module, ...rest } = action; const Action = ACTION_MODULES[module]; - /** - * In certain predefined-action modules in which a label is not required, e.g. `edit`, - * the following unique key generation fails, as it relies on the label (content). */ + // + // In certain predefined-action modules in which a label is not required, e.g. `edit`, + // the following unique key generation fails, as it relies on the label (content). const key = `${module}.${rest.label || module}`; return ; diff --git a/packages/cascara/src/ui/Table/Table.js b/packages/cascara/src/ui/Table/Table.js index d4cfcaf0c..e213b7997 100644 --- a/packages/cascara/src/ui/Table/Table.js +++ b/packages/cascara/src/ui/Table/Table.js @@ -24,18 +24,18 @@ const propTypes = { }) ), - /** Resolve record actions. - * A function that returns the actions available to the current row */ + // Resolve record actions. + // A function that returns the actions available to the current row resolveRecordActions: pt.func, }), - /** An array of objects. - * - * Every object in this array will potencially be rendered as a table row. */ + // An array of objects. + // + // Every object in this array will potencially be rendered as a table row. data: pt.arrayOf(pt.shape({})), - /** The main configuration for your table. Here you can specify the columns to display - * as well as the available actions (if any) for each row. */ + // The main configuration for your table. Here you can specify the columns to display + // as well as the available actions (if any) for each row. dataConfig: pt.shape({ actionButtonMenuIndex: pt.number, @@ -54,18 +54,18 @@ const propTypes = { ), }), - /** Event handler. - * - * An event handler you can pass to handle every event your table emits.*/ + // Event handler. + // + // An event handler you can pass to handle every event your table emits. onAction: pt.func, - /** Resolve record actions. - * A function that returns the actions available to the current row */ + // Resolve record actions. + // A function that returns the actions available to the current row resolveRecordActions: pt.func, - /** Unique ID Attribute. - * - * specifies the attribute that uniquely identifies every object in the 'data' array. */ + // Unique ID Attribute. + // + // specifies the attribute that uniquely identifies every object in the 'data' array. uniqueIdAttribute: pt.string, }; diff --git a/packages/cascara/src/ui/Table/Table.test.js b/packages/cascara/src/ui/Table/Table.test.js index 70eaadd69..f3c5ec7e9 100644 --- a/packages/cascara/src/ui/Table/Table.test.js +++ b/packages/cascara/src/ui/Table/Table.test.js @@ -8,22 +8,22 @@ import Table from './'; import { generateFakeEmployees } from '../../lib/mock/fakeData'; describe('Table', () => { - /** - * Component tree - * This test suite addresses the very basics of testing the Table UI. - * - * The first test is the snapshot, nothing special. - * - * Table actions - * An extra column is appended if any or both of these are true: - * - * a) at least one `action` is specified in `dataConfig.actions` array - * b) at least one column in the `dataConfig.display` array is editable - * - * In either cases, the extra column displays the action modules. - * - * The test `row actions` corresponds to condition a, whilst the test - * `editable records` addresses condition b. */ + // + // Component tree + // This test suite addresses the very basics of testing the Table UI. + // + // The first test is the snapshot, nothing special. + // + // Table actions + // An extra column is appended if any or both of these are true: + // + // a) at least one `action` is specified in `dataConfig.actions` array + // b) at least one column in the `dataConfig.display` array is editable + // + // In either cases, the extra column displays the action modules. + // + // The test `row actions` corresponds to condition a, whilst the test + // `editable records` addresses condition b. describe('component tree', () => { const whileTheUIisReady = async (miliseconds) => await new Promise((resolve) => setTimeout(resolve, miliseconds)); @@ -164,16 +164,16 @@ describe('Table', () => { expect(view).toMatchSnapshot(); }); - /** - * The markup generated by the table must match the dataset characteristics. - * - * All column definitions in this test suite `dataConfig.display` have a `data-testid` - * attribute which is used here to do a simple test: - * - * Does the number of (found) testIDs match the result of multiplying `datasetSize` - * by the number of columns in `dataConfig.display`? - * - * */ + // + // The markup generated by the table must match the dataset characteristics. + // + // All column definitions in this test suite `dataConfig.display` have a `data-testid` + // attribute which is used here to do a simple test: + // + // Does the number of (found) testIDs match the result of multiplying `datasetSize` + // by the number of columns in `dataConfig.display`? + // + // test('table markup vs. dataset', () => { const { display = [] } = dataConfig; render( @@ -208,14 +208,14 @@ describe('Table', () => { expect(view).toMatchSnapshot(); }); - /** - * Actions and onAction. - * - * When emitted, the `onAction` event contains two arguments, the first - * one being the element that was clicked, the second is the data of the - * row that was clicked. - * - * This test validates the Actions specified in `dataConfig.actions`. */ + // + // Actions and onAction. + // + // When emitted, the `onAction` event contains two arguments, the first + // one being the element that was clicked, the second is the data of the + // row that was clicked. + // + // This test validates the Actions specified in `dataConfig.actions`. test('with row actions', () => { const onAction = jest.fn(); @@ -258,8 +258,8 @@ describe('Table', () => { ); }); - /** - * Actions wrapped in an ActionsMenu */ + // + // Actions wrapped in an ActionsMenu test('it renders no if actionButtonMenuIndex equals button actions number', () => { const onAction = jest.fn(); @@ -288,8 +288,8 @@ describe('Table', () => { expect(allMeatBallButtons).toHaveLength(0); }); - /** - * Actions wrapped in an ActionsMenu */ + // + // Actions wrapped in an ActionsMenu test('it renders if actionButtonMenuIndex is less than the button actions number', () => { const onAction = jest.fn(); @@ -315,17 +315,17 @@ describe('Table', () => { expect(allMeatBallButtons).toHaveLength(datasetSize); }); - /** - * Editable records and onAction. - * - * The table emmits certain events depending on the actions taken by the User. - * In this scenario, the user enters the edit mode, updates the email and clicks the save button. - * - * This test validates that: - * - * 1.- the events are actualy emitted by the Table - * 2.- the data reflects the changes made by the user - * 3.- the number of buttons present in each case. */ + // + // Editable records and onAction. + // + // The table emmits certain events depending on the actions taken by the User. + // In this scenario, the user enters the edit mode, updates the email and clicks the save button. + // + // This test validates that: + // + // 1.- the events are actualy emitted by the Table + // 2.- the data reflects the changes made by the user + // 3.- the number of buttons present in each case. test('editable records', async (done) => { const testEmail = 'engineering@espressive.com'; const onAction = jest.fn(); @@ -418,16 +418,16 @@ describe('Table', () => { done(); }); - /** - * Cancelling the edition of a record. - * - * Upon exiting the edit mode via the cancel button, the Table must have emitted these events: - * - * edit.start - when clicking the edit button - * edit.cancel - when clicking the cancel button - * - * This test validates the events are actualy emitted by the Table, as well - * as the number of buttons present in each case. */ + // + // Cancelling the edition of a record. + // + // Upon exiting the edit mode via the cancel button, the Table must have emitted these events: + // + // edit.start - when clicking the edit button + // edit.cancel - when clicking the cancel button + // + // This test validates the events are actualy emitted by the Table, as well + // as the number of buttons present in each case. test('cancelling record edition', () => { const onAction = jest.fn(); diff --git a/packages/cascara/src/ui/Table/TableRow.js b/packages/cascara/src/ui/Table/TableRow.js index f95a2656a..44997baa6 100644 --- a/packages/cascara/src/ui/Table/TableRow.js +++ b/packages/cascara/src/ui/Table/TableRow.js @@ -76,7 +76,7 @@ const TableRow = ({ config = {}, record = {} }) => { const rowActions = ( {outsideActions.map(renderActionModule)} - {Boolean(insideButtonActions.length) ? ( + {insideButtonActions.length ? ( ) : null} diff --git a/packages/cascara/src/ui/Table/fixtures/ActionStack.Table.fixture.js b/packages/cascara/src/ui/Table/fixtures/ActionStack.Table.fixture.js index 306d53f60..7a3631eb2 100644 --- a/packages/cascara/src/ui/Table/fixtures/ActionStack.Table.fixture.js +++ b/packages/cascara/src/ui/Table/fixtures/ActionStack.Table.fixture.js @@ -98,7 +98,6 @@ class Fixture extends PureComponent { break; default: - return; } }; diff --git a/packages/cascara/src/ui/Table/fixtures/Conditional.Actions.fixture.js b/packages/cascara/src/ui/Table/fixtures/Conditional.Actions.fixture.js index 209b21c44..0d3fb8a18 100644 --- a/packages/cascara/src/ui/Table/fixtures/Conditional.Actions.fixture.js +++ b/packages/cascara/src/ui/Table/fixtures/Conditional.Actions.fixture.js @@ -106,19 +106,18 @@ class Fixture extends PureComponent { break; default: - return; } }; resolveRecordActions(record, actions) { return actions.reduce((actionsForRecord, action) => { switch (action.name) { - /** - * Idealy, Cascara actions would always go first - * because they go outside the ActionsMenu. - * - * Since that is something we cannot control, we - * will have to filter them out inside Cascara. */ + // + // Idealy, Cascara actions would always go first + // because they go outside the ActionsMenu. + // + // Since that is something we cannot control, we + // will have to filter them out inside Cascara. case 'edit': // do not show if record is deflected if (!record.deflected) { diff --git a/packages/cascara/src/ui/Table/fixtures/EdiTable.fixture.js b/packages/cascara/src/ui/Table/fixtures/EdiTable.fixture.js index 1086e7b38..045ee4570 100644 --- a/packages/cascara/src/ui/Table/fixtures/EdiTable.fixture.js +++ b/packages/cascara/src/ui/Table/fixtures/EdiTable.fixture.js @@ -146,7 +146,6 @@ class Fixture extends PureComponent { break; default: - return; } }; diff --git a/packages/cascara/src/ui/Table/fixtures/Export.js b/packages/cascara/src/ui/Table/fixtures/Export.js index fe71115d2..fd0aa69c4 100644 --- a/packages/cascara/src/ui/Table/fixtures/Export.js +++ b/packages/cascara/src/ui/Table/fixtures/Export.js @@ -143,7 +143,6 @@ class Fixture extends PureComponent { break; default: - return; } };