From 6816795c6d474099adb868b9409f39d8c4738dac Mon Sep 17 00:00:00 2001 From: David Lechner Date: Thu, 12 May 2022 19:17:09 -0500 Subject: [PATCH] activities: use blueprintjs tabs for keyboard a18y --- src/activities/Activities.test.tsx | 35 +----- src/activities/Activities.tsx | 173 +++++++++++++---------------- src/activities/activities.scss | 35 ++++++ src/app/App.tsx | 19 +--- src/app/app.scss | 33 ------ 5 files changed, 123 insertions(+), 172 deletions(-) create mode 100644 src/activities/activities.scss diff --git a/src/activities/Activities.test.tsx b/src/activities/Activities.test.tsx index 8b25ebfa..f436e7a2 100644 --- a/src/activities/Activities.test.tsx +++ b/src/activities/Activities.test.tsx @@ -4,9 +4,8 @@ import { cleanup } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; -import { useIsFirstRender } from 'usehooks-ts'; import { testRender } from '../../test'; -import { Activity, useActivities } from './Activities'; +import Activities, { Activity } from './Activities'; afterEach(() => { cleanup(); @@ -14,27 +13,9 @@ afterEach(() => { localStorage.clear(); }); -type TestActivityProps = { - expectedActivity: Activity; -}; - -const TestActivity: React.VoidFunctionComponent = ({ - expectedActivity, -}) => { - const [selectedActivity, activitiesComponent] = useActivities(); - - if (useIsFirstRender()) { - expect(selectedActivity).toBe(expectedActivity); - } - - return activitiesComponent; -}; - describe('Activities', () => { it('should select explorer by default', () => { - const [activities] = testRender( - , - ); + const [activities] = testRender(); const tab = activities.getByRole('tab', { name: 'File Explorer' }); @@ -47,9 +28,7 @@ describe('Activities', () => { JSON.stringify(Activity.Settings), ); - const [activities] = testRender( - , - ); + const [activities] = testRender(); const tab = activities.getByRole('tab', { name: 'Settings & Help' }); @@ -57,9 +36,7 @@ describe('Activities', () => { }); it('should select none when clicking already selected tab', () => { - const [activities] = testRender( - , - ); + const [activities] = testRender(); const explorerTab = activities.getByRole('tab', { name: 'File Explorer' }); @@ -78,9 +55,7 @@ describe('Activities', () => { }); it('should select new tab when clicking not already selected tab', () => { - const [activities] = testRender( - , - ); + const [activities] = testRender(); const explorerTab = activities.getByRole('tab', { name: 'File Explorer' }); const settingsTab = activities.getByRole('tab', { name: 'Settings & Help' }); diff --git a/src/activities/Activities.tsx b/src/activities/Activities.tsx index eb509b38..55860f86 100644 --- a/src/activities/Activities.tsx +++ b/src/activities/Activities.tsx @@ -1,10 +1,13 @@ // SPDX-License-Identifier: MIT // Copyright (c) 2022 The Pybricks Authors -import { Classes, Icon, IconName } from '@blueprintjs/core'; +import './activities.scss'; +import { Icon, Tab, Tabs } from '@blueprintjs/core'; import { useI18n } from '@shopify/react-i18n'; -import React, { useCallback, useMemo } from 'react'; +import React, { useCallback, useEffect, useRef } from 'react'; import { useLocalStorage } from 'usehooks-ts'; +import Explorer from '../explorer/Explorer'; +import Settings from '../settings/Settings'; import { I18nId } from './i18n'; /** Indicates the selected activity. */ @@ -17,95 +20,13 @@ export enum Activity { Settings = 'activity.settings', } -type ActivityTabProps = { - /** The label for the tab. */ - label: string; - /** The icon for the tab. */ - icon: IconName; - /** Controls the selected state of the tab. */ - selected: boolean; - /** Callback called when the tab is clicked. */ - onClick: () => void; -}; - -/** - * React component for tabs in {@link Activities}. - */ -const ActivityTab: React.VoidFunctionComponent = ({ - label, - icon, - selected, - onClick, -}) => { - // not using Button component so we can set role to "tab" - return ( -
c) - .join(' ')} - {...{ onClick }} - > - -
- ); -}; - -type ActivitiesProps = { - /** The currently selected activity. */ - selectedActivity: Activity; - /** Callback called when a tab is clicked. */ - onAction: (activity: Activity) => void; -}; - /** * React component that acts as a tab control to select activities. */ -const Activities: React.VoidFunctionComponent = ({ - selectedActivity, - onAction, -}) => { +const Activities: React.VoidFunctionComponent = () => { // istanbul ignore next: babel-loader rewrites this line const [i18n] = useI18n(); - return ( -
- onAction(Activity.Explorer)} - /> - onAction(Activity.Settings)} - /> -
- ); -}; - -/** - * React hook to get selected state and component. - * @returns The current selected activity (state) and the activity component. - */ -export function useActivities(): [ - selectedActivity: Activity, - activitiesComponent: React.ReactElement, -] { const [selectedActivity, setSelectedActivity] = useLocalStorage( 'activities.selectedActivity', Activity.Explorer, @@ -124,15 +45,75 @@ export function useActivities(): [ [selectedActivity, setSelectedActivity], ); - const activitiesComponent = useMemo( - () => ( - handleAction(a)} - /> - ), - [Activities, selectedActivity, handleAction], - ); + // HACK: fix keyboard focus when no tab is selected - return [selectedActivity, activitiesComponent]; -} + const tabsRef = useRef(null); + + useEffect(() => { + if (selectedActivity !== Activity.None) { + // all is well + return; + } + + // @ts-expect-error: using private property + const tablist: HTMLDivElement = tabsRef.current?.tablistElement; + + // istanbul-ignore-if: should not happen + if (!tablist) { + return; + } + + const firstTab = tablist + .getElementsByClassName('pb-activities-tablist-tab') + .item(0); + + // istanbul-ignore-if: should not happen + if (!firstTab) { + return; + } + + firstTab.setAttribute('tabindex', '0'); + }, [tabsRef, selectedActivity]); + + return ( + + + } + panel={} + panelClassName="pb-activities-tabview" + /> + + } + panel={} + panelClassName="pb-activities-tabview" + /> + + ); +}; + +export default Activities; diff --git a/src/activities/activities.scss b/src/activities/activities.scss new file mode 100644 index 00000000..24abf4d9 --- /dev/null +++ b/src/activities/activities.scss @@ -0,0 +1,35 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2022 The Pybricks Authors + +@use '@blueprintjs/core/lib/scss/variables' as bp; +@use '../variables' as pb; + +.pb-activities { + & .#{bp.$ns}-tab-list { + @include pb.background-contrast(6%); + } + + // override bluetprintjs styles + .#{bp.$ns}-tabs.#{bp.$ns}-vertical > .#{bp.$ns}-tab-list &-tablist-tab { + margin: bp.$pt-grid-size * 0.6; + padding: unset; + width: unset; + line-height: unset; + } + + &-tabview { + width: bp.$pt-grid-size * 25; + padding: bp.$pt-grid-size; + @include pb.background-contrast(0%); + + // on small screens, make the activity view an overlay instead of inline + @media screen and (max-width: pb.$narrow-screen-limit) { + position: absolute; + top: 0px; + // FIXME: this should be a variable + left: 47px; + height: 100%; + z-index: bp.$pt-z-index-overlay; + } + } +} diff --git a/src/app/App.tsx b/src/app/App.tsx index 528ad452..267981f7 100644 --- a/src/app/App.tsx +++ b/src/app/App.tsx @@ -5,10 +5,8 @@ import { Classes } from '@blueprintjs/core'; import React, { useEffect, useState } from 'react'; import SplitterLayout from 'react-splitter-layout'; import { useLocalStorage, useTernaryDarkMode } from 'usehooks-ts'; -import { Activity, useActivities } from '../activities/Activities'; +import Activities from '../activities/Activities'; import Editor from '../editor/Editor'; -import Explorer from '../explorer/Explorer'; -import Settings from '../settings/Settings'; import { useSettingIsShowDocsEnabled } from '../settings/hooks'; import StatusBar from '../status-bar/StatusBar'; import Terminal from '../terminal/Terminal'; @@ -157,18 +155,13 @@ const App: React.VFC = () => { return () => removeEventListener('keydown', listener); }, []); - const [selectedActivity, activitiesComponent] = useActivities(); - return ( -
+
e.preventDefault()} + >
- {activitiesComponent} - {selectedActivity !== Activity.None && ( -
- {selectedActivity === Activity.Explorer && } - {selectedActivity === Activity.Settings && } -
- )} + {/* need a container with position: relative; for SplitterLayout since it uses position: absolute; */}