diff --git a/src/app/content/highlights/components/ColorPicker.spec.tsx b/src/app/content/highlights/components/ColorPicker.spec.tsx index c29fb7a4483..32c030449d3 100644 --- a/src/app/content/highlights/components/ColorPicker.spec.tsx +++ b/src/app/content/highlights/components/ColorPicker.spec.tsx @@ -105,18 +105,6 @@ describe('ColorPicker', () => { expect(onChange).not.toHaveBeenCalled(); }); - it('handles having no onRemove when trashcan is clicked', () => { - const onChange = jest.fn(); - const component = renderer.create( - - ); - - const button = component.root.findByType('button'); - - button.props.onClick(); - expect(onChange).not.toHaveBeenCalled(); - }); - it('operates as a radiogroup', () => { const onChange = jest.fn(); const onRemove = jest.fn(); @@ -125,7 +113,7 @@ describe('ColorPicker', () => { ); - const rg = root.querySelector('fieldset') as HTMLFieldSetElement; + const rg = root.querySelector('fieldset > fieldset') as HTMLFieldSetElement; expect(rg).toBeTruthy(); rg?.focus(); diff --git a/src/app/content/highlights/components/ColorPicker.tsx b/src/app/content/highlights/components/ColorPicker.tsx index 15c20b14420..d20610c7ddd 100644 --- a/src/app/content/highlights/components/ColorPicker.tsx +++ b/src/app/content/highlights/components/ColorPicker.tsx @@ -82,6 +82,13 @@ function nextIdx(idx: number, itemCount: number, key: NavKeys) { return idx; } +// tslint:disable-next-line:variable-name +const FSWrapper = styled.fieldset` + border: 0; + display: flex; + flex-direction: row; +`; + // tslint:disable-next-line:variable-name const ColorPicker = ({className, ...props}: Props) => { const ref = React.useRef(null); @@ -98,11 +105,9 @@ const ColorPicker = ({className, ...props}: Props) => { const destIdx = nextIdx(idx, inputs.length, event.key as NavKeys); const el = inputs[destIdx]; - if (el) { - event.preventDefault(); - el.focus(); - el.click(); - } + event.preventDefault(); + el.focus(); + el.click(); }, [] ); @@ -120,36 +125,39 @@ const ColorPicker = ({className, ...props}: Props) => { React.useEffect(focusOnSelected, [focusOnSelected]); return ( -
- Choose highlight color - {highlightStyles.map((style) => props.multiple - ? props.selected.includes(style.label) - ? props.onChange(props.selected.filter(not(match(style.label)))) - : props.onChange([...props.selected, style.label]) - : props.color === style.label - ? props.onRemove ? props.onRemove() : null - : props.onChange(style.label)} - />)} + + Select or clear highlight color +
+ Choose highlight color + {highlightStyles.map((style) => props.multiple + ? props.selected.includes(style.label) + ? props.onChange(props.selected.filter(not(match(style.label)))) + : props.onChange([...props.selected, style.label]) + : props.color === style.label + ? props.onRemove ? props.onRemove() : null + : props.onChange(style.label)} + />)} +
{ (!hasOnRemove || props.size === 'small') ? null : } -
+ ); }; diff --git a/src/app/content/highlights/components/EditCard.spec.tsx b/src/app/content/highlights/components/EditCard.spec.tsx index 6eb7e59ba00..ba8bc81cf5a 100644 --- a/src/app/content/highlights/components/EditCard.spec.tsx +++ b/src/app/content/highlights/components/EditCard.spec.tsx @@ -120,8 +120,9 @@ describe('EditCard', () => { const data = { ...highlightData, annotation: '', + color: 'red', }; - const component = renderer.create( + const {root} = renderer.create( { ); - const picker = component.root.findByType(ColorPicker); + const picker = root.findByType(ColorPicker); + renderer.act(() => { picker.props.onRemove(); }); @@ -154,14 +156,25 @@ describe('EditCard', () => { ); const picker = component.root.findByType(ColorPicker); - renderer.act(() => { - picker.props.onRemove(); - }); - expect(editCardProps.onRemove).not.toHaveBeenCalled(); + expect(picker.props.onRemove).toBeNull(); mockSpyUser.mockClear(); }); + it('doesn\'t chain ColorPicker onRemove if there is no data', () => { + const mockSpyUser = jest.spyOn(selectAuth, 'user') + .mockReturnValue(formatUser(testAccountsUser)); + const component = renderer.create( + + + + ); + + const picker = component.root.findByType(ColorPicker); + + expect(picker.props.onRemove).toBeNull(); + mockSpyUser.mockClear(); + }); it('doesn\'t chain ColorPicker onRemove if there is a pending note', () => { highlight.getStyle.mockReturnValue('red'); const mockSpyUser = jest.spyOn(selectAuth, 'user') @@ -186,11 +199,8 @@ describe('EditCard', () => { }); const picker = component.root.findByType(ColorPicker); - renderer.act(() => { - picker.props.onRemove(); - }); - expect(editCardProps.onRemove).not.toHaveBeenCalled(); + expect(picker.props.onRemove).toBeNull(); mockSpyUser.mockClear(); }); diff --git a/src/app/content/highlights/components/EditCard.tsx b/src/app/content/highlights/components/EditCard.tsx index d771c4658a7..b4db5b9893d 100644 --- a/src/app/content/highlights/components/EditCard.tsx +++ b/src/app/content/highlights/components/EditCard.tsx @@ -253,10 +253,10 @@ function useOnRemove(props: EditCardProps, pendingAnnotation: string) { const trackDeleteHighlight = useAnalyticsEvent('deleteHighlight'); const removeAndTrack = React.useCallback( () => { - if (props.data) { - onRemove(); - trackDeleteHighlight(props.data.color); - } + const data = assertDefined(props.data, 'props.data must be defined'); + + onRemove(); + trackDeleteHighlight(data.color); }, [onRemove, props.data, trackDeleteHighlight] ); diff --git a/src/app/content/highlights/components/SummaryPopup/__snapshots__/ContextMenu.spec.tsx.snap b/src/app/content/highlights/components/SummaryPopup/__snapshots__/ContextMenu.spec.tsx.snap index 4c9cbd9582b..756502d44fe 100644 --- a/src/app/content/highlights/components/SummaryPopup/__snapshots__/ContextMenu.spec.tsx.snap +++ b/src/app/content/highlights/components/SummaryPopup/__snapshots__/ContextMenu.spec.tsx.snap @@ -136,11 +136,11 @@ exports[`ContextMenu match snapshot when open 1`] = ` overflow: hidden; } -.c19 { +.c20 { display: none; } -.c21 { +.c22 { display: none; position: absolute; height: 2rem; @@ -155,7 +155,7 @@ exports[`ContextMenu match snapshot when open 1`] = ` border-radius: 50%; } -.c22 { +.c23 { height: 1.4000000000000001rem; width: 1.4000000000000001rem; background-color: transparent; @@ -169,7 +169,7 @@ exports[`ContextMenu match snapshot when open 1`] = ` border: 1px solid #fed200; } -.c24 { +.c25 { height: 1.4000000000000001rem; width: 1.4000000000000001rem; background-color: transparent; @@ -183,7 +183,7 @@ exports[`ContextMenu match snapshot when open 1`] = ` border: 1px solid #92d101; } -.c26 { +.c27 { height: 1.4000000000000001rem; width: 1.4000000000000001rem; background-color: transparent; @@ -197,7 +197,7 @@ exports[`ContextMenu match snapshot when open 1`] = ` border: 1px solid #00c3ed; } -.c28 { +.c29 { height: 1.4000000000000001rem; width: 1.4000000000000001rem; background-color: transparent; @@ -211,7 +211,7 @@ exports[`ContextMenu match snapshot when open 1`] = ` border: 1px solid #545ec8; } -.c30 { +.c31 { height: 1.4000000000000001rem; width: 1.4000000000000001rem; background-color: transparent; @@ -225,7 +225,7 @@ exports[`ContextMenu match snapshot when open 1`] = ` border: 1px solid #de017e; } -.c16 { +.c17 { position: relative; background-color: #ffff8a; border: 1px solid #8f7700; @@ -247,19 +247,19 @@ exports[`ContextMenu match snapshot when open 1`] = ` overflow: visible; } -.c16 .c18 { +.c17 .c19 { height: 1rem; width: 1rem; } -.c16 input:focus + .c20 { +.c17 input:focus + .c21 { display: block; outline: 0.2rem auto Highlight; outline: 0.2rem auto -webkit-focus-ring-color; z-index: 1; } -.c23 { +.c24 { position: relative; background-color: #def99f; border: 1px solid #4e6f01; @@ -281,19 +281,19 @@ exports[`ContextMenu match snapshot when open 1`] = ` overflow: visible; } -.c23 .c18 { +.c24 .c19 { height: 1rem; width: 1rem; } -.c23 input:focus + .c20 { +.c24 input:focus + .c21 { display: block; outline: 0.2rem auto Highlight; outline: 0.2rem auto -webkit-focus-ring-color; z-index: 1; } -.c25 { +.c26 { position: relative; background-color: #c8f5ff; border: 1px solid #006880; @@ -315,7 +315,7 @@ exports[`ContextMenu match snapshot when open 1`] = ` overflow: visible; } -.c25 .c18 { +.c26 .c19 { height: 1rem; width: 1rem; color: #00c3ed; @@ -324,14 +324,14 @@ exports[`ContextMenu match snapshot when open 1`] = ` display: block; } -.c25 input:focus + .c20 { +.c26 input:focus + .c21 { display: block; outline: 0.2rem auto Highlight; outline: 0.2rem auto -webkit-focus-ring-color; z-index: 1; } -.c27 { +.c28 { position: relative; background-color: #cbcfff; border: 1px solid #141a3e; @@ -353,19 +353,19 @@ exports[`ContextMenu match snapshot when open 1`] = ` overflow: visible; } -.c27 .c18 { +.c28 .c19 { height: 1rem; width: 1rem; } -.c27 input:focus + .c20 { +.c28 input:focus + .c21 { display: block; outline: 0.2rem auto Highlight; outline: 0.2rem auto -webkit-focus-ring-color; z-index: 1; } -.c29 { +.c30 { position: relative; background-color: #ffc5e1; border: 1px solid #560131; @@ -387,24 +387,24 @@ exports[`ContextMenu match snapshot when open 1`] = ` overflow: visible; } -.c29 .c18 { +.c30 .c19 { height: 1rem; width: 1rem; } -.c29 input:focus + .c20 { +.c30 input:focus + .c21 { display: block; outline: 0.2rem auto Highlight; outline: 0.2rem auto -webkit-focus-ring-color; z-index: 1; } -.c17 { +.c18 { cursor: pointer; margin: 0; } -.c17 input { +.c18 input { position: absolute; opacity: 0; cursor: pointer; @@ -412,7 +412,18 @@ exports[`ContextMenu match snapshot when open 1`] = ` width: 0; } -.c15 { +.c14 { + border: 0; + display: -webkit-box; + display: -webkit-flex; + display: -ms-flexbox; + display: flex; + -webkit-flex-direction: row; + -ms-flex-direction: row; + flex-direction: row; +} + +.c16 { border: 0; outline: none; display: -webkit-box; @@ -427,7 +438,7 @@ exports[`ContextMenu match snapshot when open 1`] = ` padding: 0.8rem 0.3rem; } -.c15 legend { +.c16 legend { position: absolute; height: 1px; width: 1px; @@ -503,7 +514,7 @@ exports[`ContextMenu match snapshot when open 1`] = ` color: #818181; } -.c0 .c14 { +.c0 .c15 { margin: 0.6rem 0 0 0.6rem; } @@ -571,21 +582,21 @@ exports[`ContextMenu match snapshot when open 1`] = ` width: 100%; } -.c31 { +.c32 { width: 15px; height: 15px; margin-right: 10px; color: #424242; } -.c32 { +.c33 { width: 15px; height: 15px; margin-right: 10px; color: #424242; } -.c33 { +.c34 { width: 15px; height: 15px; margin-right: 10px; @@ -649,54 +660,26 @@ exports[`ContextMenu match snapshot when open 1`] = ` >
  • - Choose highlight color + Select or clear highlight color -
  • - } style={ Object { "focusBorder": "#8f7700", @@ -375,57 +348,57 @@ exports[`ColorPicker matches snapshot no selection 1`] = ` "passive": "#ffff8a", } } - /> - - + - + - + - + + /> + + +