Skip to content

Commit 68ca015

Browse files
aventura121aventura121
committed
FIREFLY-2119: Tooltip Design and Bug Fixes
1 parent a9b7180 commit 68ca015

3 files changed

Lines changed: 88 additions & 9 deletions

File tree

‎src/firefly/js/tables/__tests__/InputCell-test.jsx‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
/*
2+
* License information at https://github.com/Caltech-IPAC/firefly/blob/master/License.txt
3+
*/
4+
/*eslint-env node, jest */
5+
6+
import React from 'react';
7+
import '@testing-library/jest-dom';
8+
import {act, fireEvent, render, screen} from '@testing-library/react';
9+
10+
import {createInputCell, inputColumnRenderer} from '../ui/TableRenderer.js';
11+
12+
const TIP = 'syntax help';
13+
14+
const cellBuilders = {
15+
createInputCell: () => createInputCell(TIP, 10, () => ({valid: true})),
16+
inputColumnRenderer: () => inputColumnRenderer({tbl_id: 't1', cname: 'c', tooltips: TIP}),
17+
};
18+
19+
const hover = (el) => {
20+
fireEvent.mouseOver(el);
21+
act(() => jest.advanceTimersByTime(1000));
22+
};
23+
24+
describe.each(Object.keys(cellBuilders))('%s syntax tooltip', (name) => {
25+
beforeEach(() => jest.useFakeTimers());
26+
afterEach(() => jest.useRealTimers());
27+
28+
const data = [['a'], ['b']];
29+
30+
test('hover alone shows no tooltip', () => {
31+
const Cell = cellBuilders[name]();
32+
render(<Cell rowIndex={0} data={data} colIdx={0}/>);
33+
hover(screen.getByRole('textbox'));
34+
expect(screen.queryByRole('tooltip')).toBeNull();
35+
});
36+
37+
test('focus + hover shows the tooltip', () => {
38+
const Cell = cellBuilders[name]();
39+
render(<Cell rowIndex={0} data={data} colIdx={0}/>);
40+
const input = screen.getByRole('textbox');
41+
act(() => input.focus());
42+
hover(input);
43+
expect(screen.getByRole('tooltip')).toHaveTextContent(TIP);
44+
});
45+
46+
test('a cell reused for another row forgets focus that was never blurred', () => {
47+
const Cell = cellBuilders[name]();
48+
const {rerender} = render(<Cell rowIndex={0} data={data} colIdx={0}/>);
49+
act(() => screen.getByRole('textbox').focus());
50+
rerender(<Cell rowIndex={1} data={data} colIdx={0}/>); // table swaps the row; the focused input is removed, no blur
51+
hover(screen.getByRole('textbox'));
52+
expect(screen.queryByRole('tooltip')).toBeNull();
53+
});
54+
});

‎src/firefly/js/tables/ui/TableRenderer.js‎

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -155,12 +155,14 @@ export function HeaderText({val, level='body-sm', sx, ...rest}) {
155155
}
156156

157157
const blurEnter = ['blur','enter'];
158+
const syntaxTipProps = {enterDelay: 300}; // filter boxes and constraint cells can open their syntax tooltip quickly
158159

159160
function Filter({cname, onFilter, filterInfo, tbl_id}) {
160161

161162
const colGetter= () => getColumn(getTblById((tbl_id)), cname) ?? {};
162163
const col = useStoreConnector(colGetter);
163164
const [showTooltip, setShowTooltip] = useState(true);
165+
const [editing, setEditing] = useState(false); // the syntax tooltip shows only while this box has focus
164166
const dropdownEl = useRef(null);
165167

166168
useEffect(() => {
@@ -191,13 +193,19 @@ function Filter({cname, onFilter, filterInfo, tbl_id}) {
191193
validator={validator}
192194
fieldKey={name}
193195
sx={{width: 1, '--Input-radius': ''}}
194-
tooltip={showTooltip && FILTER_CONDITION_TTIPS}
196+
tooltip={editing && showTooltip && FILTER_CONDITION_TTIPS}
195197
value={filterInfoCls.getFilter(name)}
196198
onChange={onFilter}
197199
actOn={blurEnter}
198200
showWarning={false}
199201
slotProps={{
200-
input: {size:'sm', endDecorator }
202+
input: {size:'sm', endDecorator, autoComplete: 'off'},
203+
// Handle focus on the dropdown or checkboxes bubbling up and triggering the tooltip
204+
control: {
205+
onFocus: (e) => setEditing(e.target.tagName === 'INPUT' && e.currentTarget.contains(e.target)),
206+
onBlur: () => setEditing(false),
207+
},
208+
tooltip: syntaxTipProps,
201209
}}
202210
/>
203211
);
@@ -530,8 +538,14 @@ export const createInputCell = (tooltips, size = 10, validator, onChange, style)
530538
onChange && onChange(v, data, rowIndex, colIdx);
531539
};
532540

533-
return ({rowIndex, data, colIdx}) => {
534-
const val = get(data, [rowIndex, colIdx]);
541+
return function InputCell({rowIndex, data, colIdx}) {
542+
const [editing, setEditing] = useState(false); // the syntax tooltip shows only while this box has focus
543+
const [shownRow, setShownRow] = useState(rowIndex);
544+
if (shownRow !== rowIndex) { // clear editing when a scroll moves the cell to another row
545+
setShownRow(rowIndex);
546+
setEditing(false);
547+
}
548+
const val = data?.[rowIndex]?.[colIdx];
535549

536550

537551
if (val === NOT_CELL_DATA) {
@@ -543,13 +557,14 @@ export const createInputCell = (tooltips, size = 10, validator, onChange, style)
543557
key={rowIndex + '-' +colIdx}
544558
validator={(v) => validator(v, data, rowIndex, colIdx)}
545559
sx={{'.MuiInput-root':{'minHeight':'3px', 'borderRadius':4}}}
546-
tooltip={tooltips}
560+
tooltip={editing && tooltips}
547561
size={size}
548562
style={style}
549563
value={val}
550564
onChange={(v) => changeHandler(rowIndex, data, colIdx, v) }
551565
actOn={['blur','enter']}
552566
showWarning={false}
567+
slotProps={{input: {autoComplete: 'off'}, control: {onFocus: () => setEditing(true), onBlur: () => setEditing(false)}, tooltip: syntaxTipProps}}
553568
/>
554569
</Box>
555570
);
@@ -571,7 +586,13 @@ export const createInputCell = (tooltips, size = 10, validator, onChange, style)
571586
*/
572587
export const inputColumnRenderer = ({tbl_id, cname, tooltips, style={}, isReadOnly, validator, onChange, ...props}) => {
573588

574-
return ({rowIndex, data, colIdx}) => {
589+
return function InputColumnCell({rowIndex, data, colIdx}) {
590+
const [editing, setEditing] = useState(false); // the tooltip shows only while this box has focus
591+
const [shownRow, setShownRow] = useState(rowIndex);
592+
if (shownRow !== rowIndex) { // clear editing when a scroll moves the cell to another row
593+
setShownRow(rowIndex);
594+
setEditing(false);
595+
}
575596
const val = get(data, [rowIndex, colIdx]);
576597
if (isReadOnly && isReadOnly(rowIndex, data, colIdx)) {
577598
return <div style={{width: '100%', height: '100%', ...style}}>{val}</div>;
@@ -582,11 +603,12 @@ export const inputColumnRenderer = ({tbl_id, cname, tooltips, style={}, isReadOn
582603
key={rowIndex + '-' +colIdx}
583604
sx={{'.MuiInput-root':{'minHeight':'3px', 'borderRadius':4}}}
584605
validator={ validator && ((v) => validator(v, data, rowIndex, colIdx))}
585-
tooltip={tooltips}
606+
tooltip={editing && tooltips}
586607
style={{width: '100%', boxSizing: 'border-box', ...style}}
587608
value={val}
588609
onChange={makeChangeHandler(rowIndex, data, colIdx, tbl_id, cname, validator, onChange)}
589610
actOn={['blur','enter']}
611+
slotProps={{input: {autoComplete: 'off'}, control: {onFocus: () => setEditing(true), onBlur: () => setEditing(false)}, tooltip: syntaxTipProps}}
590612
/>
591613
</Box>
592614
);

‎src/firefly/js/ui/tap/TapViewType.jsx‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -548,8 +548,11 @@ function OpRender({ops, value, label='', sx, lineClamp, rowDesc='rows'}) {
548548
const op = ops.find((t) => t.value === value);
549549
if (!op) return 'none';
550550
const details= cleanUp(op.label,0);
551+
const text = op.displayLabel ?? op.label;
552+
const shown = cleanUp(text);
553+
const isCut = shown !== cleanUp(text, 0); // the tooltip only adds something when the shown text is cut short
551554
return (
552-
<Tooltip title={
555+
<Tooltip title={isCut &&
553556
<Typography level='body-sm' component='div' width='30rem'>
554557
<div dangerouslySetInnerHTML={{__html: `${details}`}}/>
555558
</Typography>
@@ -580,7 +583,7 @@ function OpRender({ops, value, label='', sx, lineClamp, rowDesc='rows'}) {
580583
WebkitBoxOrient: 'vertical',
581584
} : {}}
582585
sx={{whiteSpace:'normal', textAlign:'left'}}>
583-
<div dangerouslySetInnerHTML={{__html: `${cleanUp(op.displayLabel??op.label)}`}}/>
586+
<div dangerouslySetInnerHTML={{__html: `${shown}`}}/>
584587
</Typography>
585588
</Stack>
586589
</Tooltip>

0 commit comments

Comments
 (0)