From 8420bb9df259b71fe0517b8a0da5684865bde0b5 Mon Sep 17 00:00:00 2001 From: ahiuchingau <20424172+ahiuchingau@users.noreply.github.com> Date: Mon, 9 Nov 2020 14:34:17 -0500 Subject: [PATCH 1/4] refactor(app): display alert-circle in robot tab if calibration is marked bad closes #6604 --- app/src/components/Navbar/NavbarLink.js | 9 +++++++-- .../components/Navbar/__tests__/NavbarLink.test.js | 14 ++++++++++++++ app/src/nav/__tests__/selectors.test.js | 6 +++++- app/src/nav/selectors.js | 6 +++++- app/src/nav/types.js | 1 + 5 files changed, 32 insertions(+), 4 deletions(-) diff --git a/app/src/components/Navbar/NavbarLink.js b/app/src/components/Navbar/NavbarLink.js index a1689ce7b6e..f5c4c0b4ed1 100644 --- a/app/src/components/Navbar/NavbarLink.js +++ b/app/src/components/Navbar/NavbarLink.js @@ -58,15 +58,18 @@ export function NavbarLink(props: NavbarLinkProps): React.Node { iconName, disabledReason, notificationReason, + warningReason, ...styleProps } = props const [targetProps, tooltipProps] = useHoverTooltip({ placement: TOOLTIP_RIGHT, }) - const tooltipContents = disabledReason ?? notificationReason ?? null + const tooltipContents = + disabledReason ?? notificationReason ?? warningReason ?? null const hasNotification = Boolean(notificationReason) const isDisabled = Boolean(disabledReason) + const hasWarning = Boolean(warningReason) const LinkComponent = isDisabled ? 'div' : NavLink const linkProps = isDisabled ? styleProps : { to: path, ...styleProps } @@ -84,7 +87,9 @@ export function NavbarLink(props: NavbarLinkProps): React.Node { > diff --git a/app/src/components/Navbar/__tests__/NavbarLink.test.js b/app/src/components/Navbar/__tests__/NavbarLink.test.js index 5b27f224a6a..de03d2c9eb9 100644 --- a/app/src/components/Navbar/__tests__/NavbarLink.test.js +++ b/app/src/components/Navbar/__tests__/NavbarLink.test.js @@ -108,6 +108,20 @@ describe('NavbarLink component', () => { expect(icon.prop('childName')).toBe('circle') }) + it('should render a warning if there is a reason to', () => { + const wrapper = render({ + id: 'foo', + path: '/foo', + title: 'Foo', + iconName: 'alert', + warningReason: 'hello', + }) + const icon = wrapper.find(NotificationIcon) + + expect(icon.prop('name')).toBe('alert') + expect(icon.prop('childName')).toBe('alert-circle') + }) + it('should render the link title', () => { const wrapper = render({ id: 'foo', diff --git a/app/src/nav/__tests__/selectors.test.js b/app/src/nav/__tests__/selectors.test.js index d6153818b17..7e98faca37f 100644 --- a/app/src/nav/__tests__/selectors.test.js +++ b/app/src/nav/__tests__/selectors.test.js @@ -97,6 +97,7 @@ const EXPECTED_ROBOTS = { title: 'Robot', iconName: 'ot-connect', notificationReason: null, + warningReason: null, } const EXPECTED_UPLOAD = { @@ -295,7 +296,10 @@ describe('nav selectors', () => { mockGetDeckCalibrationStatus.mockReturnValue(DECK_CAL_STATUS_IDENTITY) }, expected: [ - EXPECTED_ROBOTS, + { + ...EXPECTED_ROBOTS, + warningReason: expect.stringMatching(/Robot calibration recommended/), + }, { ...EXPECTED_UPLOAD, disabledReason: expect.stringMatching(/calibrate your deck/i), diff --git a/app/src/nav/selectors.js b/app/src/nav/selectors.js index 974a893f2c8..7db90996595 100644 --- a/app/src/nav/selectors.js +++ b/app/src/nav/selectors.js @@ -41,6 +41,7 @@ const CALIBRATE_DECK_TO_PROCEED = 'Calibrate your deck to proceed' const APP_UPDATE_AVAILABLE = 'An app update is available' const DRIVER_UPDATE_AVAILABLE = 'A driver update is available' const ROBOT_UPDATE_AVAILABLE = 'A robot software update is available' +const ROBOT_CALIBRATION_RECOMMENDED = 'Robot calibration recommended' const getConnectedRobotPipettesMatch = (state: State): boolean => { const connectedRobot = getConnectedRobot(state) @@ -94,13 +95,16 @@ const getRunDisabledReason: State => string | null = createSelector( ) export const getRobotsLocation: State => NavLocation = createSelector( + getConnectedRobot, getConnectedRobotUpdateAvailable, - update => ({ + getDeckCalibrationOk, + (robot, update, deckCalOk) => ({ id: 'robots', path: '/robots', title: ROBOT, iconName: 'ot-connect', notificationReason: update ? ROBOT_UPDATE_AVAILABLE : null, + warningReason: robot && !deckCalOk ? ROBOT_CALIBRATION_RECOMMENDED : null, }) ) diff --git a/app/src/nav/types.js b/app/src/nav/types.js index aca8c327bf2..b009ceb762f 100644 --- a/app/src/nav/types.js +++ b/app/src/nav/types.js @@ -9,6 +9,7 @@ export type NavLocation = {| iconName: IconName, disabledReason?: string | null, notificationReason?: string | null, + warningReason?: string | null, |} export type SubnavLocation = {| From c0a9e3ee08c736ae65bfc6f4fa0ffb94919212ba Mon Sep 17 00:00:00 2001 From: ahiuchingau <20424172+ahiuchingau@users.noreply.github.com> Date: Mon, 9 Nov 2020 15:29:19 -0500 Subject: [PATCH 2/4] check attached pip offset & tip length as well --- app/src/nav/selectors.js | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/app/src/nav/selectors.js b/app/src/nav/selectors.js index 7db90996595..f63b58a8091 100644 --- a/app/src/nav/selectors.js +++ b/app/src/nav/selectors.js @@ -5,12 +5,19 @@ import { getConnectedRobot } from '../discovery' import { getProtocolPipettesMatching, getProtocolPipettesCalibrated, + getAttachedPipetteCalibrations, + PIPETTE_MOUNTS, } from '../pipettes' import { selectors as RobotSelectors } from '../robot' import { UPGRADE, getBuildrootUpdateAvailable } from '../buildroot' import { getAvailableShellUpdate } from '../shell' import { getU2EWindowsDriverStatus, OUTDATED } from '../system-info' -import { getDeckCalibrationStatus, DECK_CAL_STATUS_OK } from '../calibration' +import { + getDeckCalibrationStatus, + DECK_CAL_STATUS_OK, + getPipetteOffsetCalibrations, + getDeckCalibrationData, +} from '../calibration' import type { State } from '../types' import type { NavLocation } from './types' @@ -76,6 +83,25 @@ const getDeckCalibrationOk = (state: State): boolean => { return deckCalStatus === DECK_CAL_STATUS_OK } +const getRobotCalibrationOk = (state: State): boolean => { + const connectedRobot = getConnectedRobot(state) + if (!connectedRobot) return false + + const deckCalOk = getDeckCalibrationOk(state) + const pipCal = getAttachedPipetteCalibrations(state, connectedRobot.name) + for (const m of PIPETTE_MOUNTS) { + if (pipCal) { + if ( + pipCal[m]?.offset?.status?.markedBad || + pipCal[m]?.tipLength?.status?.markedBad + ) { + return false + } + } + } + return deckCalOk +} + const getRunDisabledReason: State => string | null = createSelector( getConnectedRobot, RobotSelectors.getSessionIsLoaded, @@ -97,14 +123,14 @@ const getRunDisabledReason: State => string | null = createSelector( export const getRobotsLocation: State => NavLocation = createSelector( getConnectedRobot, getConnectedRobotUpdateAvailable, - getDeckCalibrationOk, - (robot, update, deckCalOk) => ({ + getRobotCalibrationOk, + (robot, update, robotCalOk) => ({ id: 'robots', path: '/robots', title: ROBOT, iconName: 'ot-connect', notificationReason: update ? ROBOT_UPDATE_AVAILABLE : null, - warningReason: robot && !deckCalOk ? ROBOT_CALIBRATION_RECOMMENDED : null, + warningReason: robot && !robotCalOk ? ROBOT_CALIBRATION_RECOMMENDED : null, }) ) From f433549d5a2e59c1c8d768b068987c5e5b3d3d79 Mon Sep 17 00:00:00 2001 From: ahiuchingau <20424172+ahiuchingau@users.noreply.github.com> Date: Mon, 9 Nov 2020 15:40:35 -0500 Subject: [PATCH 3/4] fix lint error --- app/src/nav/selectors.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/app/src/nav/selectors.js b/app/src/nav/selectors.js index f63b58a8091..affeca08569 100644 --- a/app/src/nav/selectors.js +++ b/app/src/nav/selectors.js @@ -12,12 +12,7 @@ import { selectors as RobotSelectors } from '../robot' import { UPGRADE, getBuildrootUpdateAvailable } from '../buildroot' import { getAvailableShellUpdate } from '../shell' import { getU2EWindowsDriverStatus, OUTDATED } from '../system-info' -import { - getDeckCalibrationStatus, - DECK_CAL_STATUS_OK, - getPipetteOffsetCalibrations, - getDeckCalibrationData, -} from '../calibration' +import { getDeckCalibrationStatus, DECK_CAL_STATUS_OK } from '../calibration' import type { State } from '../types' import type { NavLocation } from './types' From 9caeebfbf4dc9df6b2e79e8898c59b75d087b3c4 Mon Sep 17 00:00:00 2001 From: ahiuchingau <20424172+ahiuchingau@users.noreply.github.com> Date: Mon, 9 Nov 2020 17:13:21 -0500 Subject: [PATCH 4/4] make requested changes --- app/src/components/Navbar/NavbarLink.js | 2 +- .../Navbar/__tests__/NavbarLink.test.js | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/app/src/components/Navbar/NavbarLink.js b/app/src/components/Navbar/NavbarLink.js index f5c4c0b4ed1..5444bcde8c2 100644 --- a/app/src/components/Navbar/NavbarLink.js +++ b/app/src/components/Navbar/NavbarLink.js @@ -88,7 +88,7 @@ export function NavbarLink(props: NavbarLinkProps): React.Node { { expect(icon.prop('childName')).toBe('alert-circle') }) + it('should render a warning icon if both notification and warning reasons exist', () => { + const wrapper = render({ + id: 'foo', + path: '/foo', + title: 'Foo', + iconName: 'alert', + notificationReason: 'hi', + warningReason: 'hello', + }) + const icon = wrapper.find(NotificationIcon) + + expect(icon.prop('name')).toBe('alert') + expect(icon.prop('childName')).toBe('alert-circle') + }) + it('should render the link title', () => { const wrapper = render({ id: 'foo',