diff --git a/.eslintrc.js b/.eslintrc.js index d1aa92d4..04cf3aa1 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -66,6 +66,7 @@ module.exports = { 'import/no-cycle': 'off', 'no-underscore-dangle': 'off', 'no-nested-ternary': 'off', + 'jsx-a11y/tabindex-no-positive': 'off', 'no-bitwise': 'warn', }, }; diff --git a/e2e-tests/alttext.spec.ts b/e2e-tests/alttext.spec.ts index d11b8aad..fa59051a 100644 --- a/e2e-tests/alttext.spec.ts +++ b/e2e-tests/alttext.spec.ts @@ -41,14 +41,10 @@ test.beforeEach(async ({ page }) => { test('Alt Text', async ({ page }) => { await page.goto('http://localhost:3000/?workspace=Upset+Examples&table=simpsons&sessionId=193'); - const altTextSidebarButton = await page.getByLabel('Alt Text Sidebar', { exact: true }); - await expect(altTextSidebarButton).toBeVisible(); - await altTextSidebarButton.click(); - - const altTextSidebar = await page.getByLabel('Accessibility Sidebar', { exact: true }); + const altTextSidebar = await page.getByLabel('Alt Text and Plot Information Sidebar', { exact: true }); await expect(altTextSidebar).toBeVisible(); - const altTextHeading = await page.getByRole('heading', { name: 'Accessibility Sidebar' }); + const altTextHeading = await page.getByRole('heading', { name: 'Text Description' }); await expect(altTextHeading).toBeVisible(); /// ///////////////// @@ -76,7 +72,7 @@ test('Alt Text', async ({ page }) => { // Plot Information /// ///////////////// // Editing - const editPlotInformationButton = await page.getByLabel('Plot Information'); + const editPlotInformationButton = await page.getByRole('button', { name: 'Add Plot Information' }); await expect(editPlotInformationButton).toBeVisible(); await editPlotInformationButton.click(); @@ -114,23 +110,23 @@ test('Alt Text', async ({ page }) => { await expect(plotInformationOutput).toBeVisible(); await expect(plotInformationOutput).toHaveText('This UpSet plot shows Test dataset description. The sets are Test sets value. The items are Test items value.'); - await expect(page.getByLabel('Accessibility Sidebar')).toContainText('This UpSet plot shows Test dataset description. The sets are Test sets value. The items are Test items value.'); + await expect(page.getByLabel('Alt Text and Plot Information Sidebar')).toContainText('This UpSet plot shows Test dataset description. The sets are Test sets value. The items are Test items value.'); await page.getByRole('button', { name: 'Save' }).click(); // Display - await expect(page.getByLabel('Accessibility Sidebar')).toContainText('upset plot'); - await expect(page.getByLabel('Accessibility Sidebar')).toContainText('upset plot caption'); - await expect(page.getByLabel('Accessibility Sidebar')).toContainText('This UpSet plot shows Test dataset description. The sets are Test sets value. The items are Test items value.'); + await expect(page.getByLabel('Alt Text and Plot Information Sidebar')).toContainText('upset plot'); + await expect(page.getByLabel('Alt Text and Plot Information Sidebar')).toContainText('upset plot caption'); + await expect(page.getByLabel('Alt Text and Plot Information Sidebar')).toContainText('This UpSet plot shows Test dataset description. The sets are Test sets value. The items are Test items value.'); /// ///////////////// // Short Description /// ///////////////// await expect(page.getByText('This is an UpSet plot')) .toContainText('This is an UpSet plot which shows set intersection of 6 sets out of 6 sets and the largest intersection is School, and Male (3). The plot is sorted by size and 12 non-empty intersections are shown.'); - await page.getByLabel("Display Long Description").click(); - await page.getByLabel("Display Long Description").click(); + await page.getByRole('button', { name: 'Show More' }).click(); + await page.getByRole('button', { name: 'Show Less' }).click(); await expect(page.getByText('This is an UpSet plot which')).toBeVisible(); - await page.getByLabel("Display Long Description").click(); + await page.getByRole('button', { name: 'Show More' }).click(); /// ///////////////// // Alt Text Output @@ -187,31 +183,27 @@ test('Alt Text', async ({ page }) => { /// ///////////////// // User Description Test /// ///////////////// - await page.getByLabel('View User Description(s)').check(); - await expect(page.getByLabel('Accessibility Sidebar')).toContainText('No user-generated description available.'); - await page.getByLabel('Display Long Description').uncheck(); - await expect(page.getByLabel('Accessibility Sidebar')).toContainText('No user-generated description available.'); - // User short - await page.getByRole('button', { name: 'Edit Text Description' }).click(); + await page.getByRole('button', { name: 'Show Less' }).click(); + await page.getByLabel('Alt Text Description Editor').click(); await page.getByText('This is an UpSet plot which').click(); await page.getByText('This is an UpSet plot which').fill('This is an UpSet plot which shows set intersection of 6 sets out of 6 sets and the abcdefg largest intersection is School, and Male (3). The plot is sorted by size and 12 non-empty intersections are shown.'); await page.getByRole('button', { name: 'Save' }).click(); - await expect(page.getByLabel('Accessibility Sidebar')).toContainText('This is an UpSet plot which shows set intersection of 6 sets out of 6 sets and the abcdefg largest intersection is School, and Male (3). The plot is sorted by size and 12 non-empty intersections are shown.'); - + await expect(page.getByLabel('Alt Text and Plot Information Sidebar')).toContainText('This is an UpSet plot which shows set intersection of 6 sets out of 6 sets and the abcdefg largest intersection is School, and Male (3). The plot is sorted by size and 12 non-empty intersections are shown.'); + // User long, switching while editing - await page.getByRole('button', { name: 'Edit Text Description' }).click(); - await page.getByLabel('Display Long Description').check(); + await page.getByLabel('Alt Text Description Editor').click(); + await page.getByRole('button', { name: 'Show More' }).click(); await page.getByText('# UpSet Introduction This is').click(); await page.getByText('# UpSet Introduction This is').fill('# UpSet Introduction\nThis is an UpSet plot that visualizes set intersection. To learn about UpSet plots, visit https://upset.app.\n\n# Dataset Properties\nThe dataset contains 6 sets and 44 elements, of which 6 sets are shown in the plot.\n\n# Set Properties\nThe set sizes are diverging a lot, ranging from 3 to 18. The largest set is Male with 18 elements, followed by School with 6, Duff Fan with 6, Evil with 6, Power Plant with 5, and Blue Hair with 3.\n\n# Intersection Properties afdegb\nThe plot is sorted by size in ascending order. There are 12 non-empty intersections, all of which are shown in the plot. The largest 5 intersections are School, and Male (3), the empty intersection (3), Just Male (3), Duff Fan, Male, and Power Plant (3), and Evil, and Male (2).\n\n# Statistical Information\nThe average intersection size is 2, and the median is 2. The 90th percentile is 3, and the 10th percentile is 1. The largest set, Male, is present in 75.0% of all non-empty intersections. The smallest set, Blue Hair, is present in 16.7% of all non-empty intersections.\n\n# Trend Analysis\n The intersection sizes start from a value of 1 and then drastically rise up to 3. The empty intersection is present with a size of 3. An all set intersection is not present. The individual set intersections are large in size. The low degree set intersections lie in medium and largest sized intersections. The medium degree set intersections can be seen among small sized intersections. No high order intersections are present.\n\n'); await page.getByRole('button', { name: 'Save' }).click(); await expect(page.locator('[id="Intersection\\ Properties\\ afdegb"]')).toContainText('Intersection Properties afdegb'); - + // Resetting descriptions - await page.getByRole('button', { name: 'Edit Text Description' }).click(); + await page.getByLabel('Alt Text Description Editor').click(); await page.getByRole('button', { name: 'Reset Descriptions' }).click(); await page.getByRole('button', { name: 'Save' }).click(); await expect(page.locator('[id="Intersection\\ Properties"]')).toContainText('Intersection Properties'); - await page.getByLabel('Display Long Description').uncheck(); - await expect(page.getByLabel('Accessibility Sidebar')).toContainText('This is an UpSet plot which shows set intersection of 6 sets out of 6 sets and the largest intersection is School, and Male (3). The plot is sorted by size and 12 non-empty intersections are shown.'); + await page.getByRole('button', { name: 'Show Less' }).click(); + await expect(page.getByLabel('Alt Text and Plot Information Sidebar')).toContainText('This is an UpSet plot which shows set intersection of 6 sets out of 6 sets and the largest intersection is School, and Male (3). The plot is sorted by size and 12 non-empty intersections are shown.'); }); diff --git a/packages/app/src/atoms/altTextSidebarAtom.ts b/packages/app/src/atoms/altTextSidebarAtom.ts index 5c2a0a22..de5f7ac8 100644 --- a/packages/app/src/atoms/altTextSidebarAtom.ts +++ b/packages/app/src/atoms/altTextSidebarAtom.ts @@ -2,5 +2,5 @@ import { atom } from 'recoil'; export const altTextSidebarAtom = atom({ key: 'alt-text-sidebar-state', - default: false, + default: true, }); diff --git a/packages/app/src/components/Header/index.tsx b/packages/app/src/components/Header/index.tsx index 2e927bc1..b2f5b347 100644 --- a/packages/app/src/components/Header/index.tsx +++ b/packages/app/src/components/Header/index.tsx @@ -50,7 +50,7 @@ const Header = ({ data }: { data: any }) => { * "Tab indicies" refers to the number of tabIndex properties on elements in the sidebar * @see AltTextSidebar to count the number of tab indices used */ - const ALTTEXT_SIDEBAR_TABS = (isAltTextSidebarOpen ? 17 : 0); + const ALTTEXT_SIDEBAR_TABS = (isAltTextSidebarOpen ? 18 : 0); const handleImportModalClose = () => { setShowImportModal(false); @@ -187,10 +187,10 @@ const Header = ({ data }: { data: any }) => { setIsAltTextSidebarOpen(true); } }} - aria-label='Alt Text Sidebar' + aria-label='Text Descriptions (Alt Text) Sidebar' tabIndex={2} > - Text Description + Text Descriptions void; @@ -61,30 +60,47 @@ const initialDrawerWidth = 450; * @returns {JSX.Element} The AltTextSidebar component. */ export const AltTextSidebar: FC = ({ open, close, generateAltText }) => { + /** + * State + */ + const { actions }: {actions: UpsetActions} = useContext(ProvenanceContext); - const currState = useRecoilValue(upsetConfigAtom); - const [altText, setAltText] = useState(null); const [textGenErr, setTextGenErr] = useState(false); - - // States for editing the alt text const [textEditing, setTextEditing] = useState(false); const [userLongText, setUserLongText] = useState(currState.userAltText?.longDescription); const [userShortText, setUserShortText] = useState(currState.userAltText?.shortDescription); const [useLong, setUseLong] = useState(false); + const [plotInfoEditing, setPlotInfoEditing] = useState(false); + const plotInfo = useRecoilValue(plotInformationSelector); + + /** + * Functions + */ /** * Handler for when the save button is clicked */ - function saveButtonClick() { + const saveButtonClick: () => void = useCallback(() => { setTextEditing(false); - if (!currState.useUserAlt) - actions.setUseUserAltText(true); - if (currState.userAltText?.shortDescription !== userShortText - || currState.userAltText?.longDescription !== userLongText) - actions.setUserAltText({shortDescription: userShortText ?? "", longDescription: userLongText ?? ""}); - } + if (!currState.useUserAlt) actions.setUseUserAltText(true); + if (currState.userAltText?.shortDescription !== userShortText + || currState.userAltText?.longDescription !== userLongText) { actions.setUserAltText({ shortDescription: userShortText ?? '', longDescription: userLongText ?? '' }); } + }, [currState, userShortText, userLongText, actions]); + + /** + * Sets text editing to true and sets default user alttexts if necessary + */ + const enableTextEditing: () => void = useCallback(() => { + setTextEditing(true); + if (!currState.userAltText?.shortDescription) setUserShortText(altText?.shortDescription); + if (!currState.userAltText?.longDescription) setUserLongText(altText?.longDescription); + }, [currState, altText]); + + /** + * Effects + */ // values added as a dependency here indicate values which are usable to the alt-text generator API call // When new options are added to the alt-text API, they should be added here as well @@ -98,28 +114,47 @@ export const AltTextSidebar: FC = ({ open, close, generateAltText }) => { setTextGenErr(msg); } } - + generate(); }, [currState]); + /** + * Constants + */ + + /** + * Number of tab indicies used by the PlotInformation component + * @see PlotInformation to count the number of tab indices used + */ + const PLOT_INFO_TABS = 7; + /** + * The tab index, in this component, of the plot information component + */ + const PLOT_INFO_TAB_INDEX = 9; + // Current alt text to display to the user const displayAltText: string | undefined = useMemo(() => { - if (textEditing) - if (useLong) return userLongText ?? altText?.longDescription - else return userShortText ?? altText?.shortDescription - else if (useLong) return currState.useUserAlt ? userLongText : altText?.longDescription - else return currState.useUserAlt ? userShortText : altText?.shortDescription; - }, [useLong, userLongText, userShortText, altText?.shortDescription, altText?.longDescription, currState.useUserAlt]); - + if (useLong) return userLongText ?? altText?.longDescription ?? ''; + return userShortText ?? altText?.shortDescription ?? ''; + }, [useLong, userLongText, userShortText, altText?.shortDescription, altText?.longDescription]); + const divider = - + aria-hidden + />; + + /** + * Whether to display the plot information section + */ + const displayPlotInfo: boolean = useMemo( + () => plotInfoEditing || (currState.plotInformation && Object.values(currState.plotInformation).some((v) => !!v)), + [currState.plotInformation, plotInfoEditing], + ); + return ( = ({ open, close, generateAltText }) => { open={open} onClose={close} variant="persistent" - aria-label="Accessibility Sidebar" + aria-label="Alt Text and Plot Information Sidebar" sx={{ width: (open) ? initialDrawerWidth : 0, flexShrink: 0, @@ -142,128 +177,131 @@ export const AltTextSidebar: FC = ({ open, close, generateAltText }) => { >

- - Accessibility Sidebar - - {divider} - - - Description - + + + {displayPlotInfo ? plotInfo.title ?? 'Editing Plot Information' : 'Text Description'} + + + {divider} - - {!textGenErr ? (<> - { - if (currState.useUserAlt !== ev.target.checked) - actions.setUseUserAltText(ev.target.checked); - if (!ev.target.checked) { - setTextEditing(false); - } - }} - /> - } - labelPlacement="start" - /> - -
- { - setUseLong(ev.target.checked); - }} - /> - } - labelPlacement="start" + {displayPlotInfo && + // We only want to display plotInfo if the user is editing OR if they've entered some field other than title + (plotInfoEditing || Object.entries(plotInfo).filter(([k, _]) => k !== 'title').some(([_, v]) => !!v)) ? ( + - - ) : ( - {textGenErr} - )} - {textEditing ? (<> - + ) : ( -
- useLong ? setUserLongText(e.target.value) : setUserShortText(e.target.value)} - value={(displayAltText)} - tabIndex={5} - /> -
- ) : ( -
{ - setTextEditing(true); - if (!currState.userAltText?.shortDescription) - setUserShortText(altText?.shortDescription); - if (!currState.userAltText?.longDescription) - setUserLongText(altText?.longDescription); - }} - tabIndex={3} + onClick={() => setPlotInfoEditing(true)} + tabIndex={PLOT_INFO_TAB_INDEX} > - - + +
+ (useLong ? setUserLongText(e.target.value) : setUserShortText(e.target.value))} + value={(displayAltText)} + tabIndex={6} + aria-flowto="saveAltTextButton" + /> +
+ + ) : ( + setTextEditing(true)} - tabIndex={4} - >Edit Text Description -
+ tabIndex={3} + > + + + + ) )} +
); }; - \ No newline at end of file diff --git a/packages/upset/src/components/custom/PlotInformation.tsx b/packages/upset/src/components/custom/PlotInformation.tsx index 0f94aaea..b748e96a 100644 --- a/packages/upset/src/components/custom/PlotInformation.tsx +++ b/packages/upset/src/components/custom/PlotInformation.tsx @@ -1,4 +1,5 @@ import { + Alert, Box, Button, Icon, @@ -7,7 +8,9 @@ import { } from '@mui/material'; import EditIcon from '@mui/icons-material/Edit'; import { useRecoilValue } from 'recoil'; -import { useContext, useState } from 'react'; +import { + useContext, useState, useCallback, +} from 'react'; import { plotInformationSelector } from '../../atoms/config/plotInformationAtom'; import { ProvenanceContext } from '../Root'; @@ -21,13 +24,17 @@ type Props = { */ onSave?: () => void; /** - * The JSX element to be used as a divider. + * Callback to set the editing state. */ - divider: JSX.Element; + setEditing: (editing: boolean) => void; /** - * The starting tab index for the component. Uses up to 5 additional indices + * The starting tab index for the component. Uses up to 6 additional indices */ tabIndex: number; + /** + * Whether the component is in editing mode by default + */ + editing: boolean; } /** @@ -35,13 +42,19 @@ type Props = { * Uses up to 5 tab indices, starting from @param tabIndex * @param Props @see @type Props */ -export const PlotInformation = ({ onSave, divider, tabIndex }: Props) => { +export const PlotInformation = ({ + onSave, setEditing, tabIndex, editing, +}: Props) => { /** * Width of the titles for all the fields in %. * Field entry boxes will occupy the rest of the space */ const fieldTitleWidth = 30; + /** + * Constants + */ + const plotInfoItem = { display: 'flex', width: '100%', @@ -56,42 +69,47 @@ export const PlotInformation = ({ onSave, divider, tabIndex }: Props) => { color: 'GrayText', width: `${fieldTitleWidth}%`, }; - const placeholderText = { description: 'description of this dataset, eg: "movie genres and ratings"', sets: 'name for the sets in this data, eg: "movie genres"', items: 'name for the items in this data, eg: "movies"', }; + /** + * State + */ + const plotInformationState = useRecoilValue(plotInformationSelector); const [plotInformation, setPlotInformation] = useState(plotInformationState); const { actions } = useContext(ProvenanceContext); - const [editing, setEditing] = useState(false); + /** + * Functions + */ /** * Generates plot information string * @returns string Plot information description */ - const generatePlotInformationText: () => string = () => { + const generatePlotInformationText: () => string = useCallback(() => { // return default string if there are no values filled in - if (Object.values(plotInformation).filter((a) => a?.length > 0).length === 0) { + if (Object.values(plotInformation).filter((a) => a && a.length > 0).length === 0) { return `This UpSet plot shows ${placeholderText.description}. The sets are ${placeholderText.sets}. The items are ${placeholderText.items}`; } let str: string = ''; - if (plotInformation.description !== '') { + if (plotInformation.description) { str += `This UpSet plot shows ${plotInformation.description}. `; } - if (plotInformation.sets !== '') { + if (plotInformation.sets) { str += `The sets are ${plotInformation.sets}. `; } - if (plotInformation.items !== '') { + if (plotInformation.items) { str += `The items are ${plotInformation.items}.`; } return str; - }; + }, [plotInformation, placeholderText]); /** * Commits changes to the plot information if the new state is different from the old state. @@ -113,32 +131,29 @@ export const PlotInformation = ({ onSave, divider, tabIndex }: Props) => { !editing ? ( setEditing(true)} sx={{ - cursor: 'pointer', border: '2px solid white', // Prevent resize on hover padding: '.2em', - '&:hover': { - border: '2px inset #ddd', - }, }} >
- - {plotInformation.title ?? '[Title]'} - + {plotInformation.caption}
- {divider} - {plotInformation.caption ?? '[Caption]'}
{generatePlotInformationText()} @@ -158,11 +173,10 @@ export const PlotInformation = ({ onSave, divider, tabIndex }: Props) => { fullWidth variant="standard" style={{ marginBottom: '5px' }} - value={plotInformation.title} + value={plotInformation.title ?? ''} onChange={(e) => setPlotInformation({ ...plotInformation, title: e.target.value })} placeholder="Title" inputProps={{ - disableUnderline: true, style: { padding: '1px', height: '1.4em', @@ -181,11 +195,14 @@ export const PlotInformation = ({ onSave, divider, tabIndex }: Props) => { // We need to override the default overflow prop (hidden), then still deny x scrolling style: { height: '4em', overflow: 'auto', overflowX: 'hidden' }, }} - value={plotInformation.caption} + value={plotInformation.caption ?? ''} onChange={(e) => setPlotInformation({ ...plotInformation, caption: e.target.value })} placeholder="Caption" /> + + Providing the following information will help us improve auto-generated text descriptions + @@ -197,7 +214,7 @@ export const PlotInformation = ({ onSave, divider, tabIndex }: Props) => { sx={{ width: `${100 - fieldTitleWidth}%` }} multiline InputLabelProps={{ shrink: true }} - value={plotInformation.description} + value={plotInformation.description ?? ''} fullWidth maxRows={8} placeholder={`${placeholderText.description}`} @@ -215,7 +232,7 @@ export const PlotInformation = ({ onSave, divider, tabIndex }: Props) => { sx={{ width: `${100 - fieldTitleWidth}%` }} multiline InputLabelProps={{ shrink: true }} - value={plotInformation.sets} + value={plotInformation.sets ?? ''} fullWidth maxRows={8} placeholder={`${placeholderText.sets}`} @@ -233,7 +250,7 @@ export const PlotInformation = ({ onSave, divider, tabIndex }: Props) => { sx={{ width: `${100 - fieldTitleWidth}%` }} multiline InputLabelProps={{ shrink: true }} - value={plotInformation.items} + value={plotInformation.items ?? ''} fullWidth maxRows={8} placeholder={`${placeholderText.items}`}