Commit beef62e0 authored by Lyza Danger Gardner's avatar Lyza Danger Gardner Committed by Lyza Gardner

AnontationShareControl: get group from store instead of prop

Refactor to relieve a long-standing FIXME in the
`AnnotationShareControl` component, which pre-dates the completion of
the conversion of the app's UI to preact and the full complement of
available store modules.

As groups and annotations are loaded separately, we cannot guarantee
that group metadata is loaded for a given annotation's group (ID).

Attempt to retrieve the annotation's group from the store, and if it
is undefined, don't render the component.

See https://github.com/hypothesis/client/issues/1542
parent 43918612
......@@ -58,7 +58,6 @@ function AnnotationActionBar({
}: AnnotationActionBarProps) {
const store = useSidebarStore();
const userProfile = store.profile();
const annotationGroup = store.getGroup(annotation.group);
const isLoggedIn = store.isLoggedIn();
// Is the current user allowed to take the given `action` on this annotation?
......@@ -128,11 +127,7 @@ function AnnotationActionBar({
)}
<IconButton icon={ReplyIcon} title="Reply" onClick={onReplyClick} />
{shareLink && (
<AnnotationShareControl
annotation={annotation}
group={annotationGroup}
shareUri={shareLink}
/>
<AnnotationShareControl annotation={annotation} shareUri={shareLink} />
)}
{showFlagAction && !annotation.flagged && (
<IconButton
......
......@@ -11,12 +11,13 @@ import classnames from 'classnames';
import { useEffect, useRef, useState } from 'preact/hooks';
import { isIOS } from '../../../shared/user-agent';
import type { Annotation, Group } from '../../../types/api';
import type { Annotation } from '../../../types/api';
import { isShareableURI } from '../../helpers/annotation-sharing';
import { isPrivate } from '../../helpers/permissions';
import { withServices } from '../../service-context';
import type { ToastMessengerService } from '../../services/toast-messenger';
import { useSidebarStore } from '../../store';
import { copyText } from '../../util/copy-to-clipboard';
import MenuArrow from '../MenuArrow';
......@@ -26,14 +27,6 @@ export type AnnotationShareControlProps = {
/** The annotation in question */
annotation: Annotation;
/**
* Group that the annotation is in. If missing, this component will not
* render.
* FIXME: Refactor after root cause is addressed. See
* https://github.com/hypothesis/client/issues/1542
*/
group?: Group;
/** The URI to view the annotation on its own */
shareUri: string;
......@@ -55,9 +48,11 @@ function selectionOverflowsInputElement() {
function AnnotationShareControl({
annotation,
toastMessenger,
group,
shareUri,
}: AnnotationShareControlProps) {
const store = useSidebarStore();
const group = store.getGroup(annotation.group);
const annotationIsPrivate = isPrivate(annotation.permissions);
const inContextAvailable = isShareableURI(annotation.uri);
const inputRef = useRef<HTMLInputElement | null>(null);
......@@ -84,8 +79,9 @@ function AnnotationShareControl({
}
}, [isOpen]);
// FIXME: See https://github.com/hypothesis/client/issues/1542
if (!group) {
// This can happen if groups have just been unloaded but annotations have
// not yet been unloaded, e.g. on logout if a private group was focused
return null;
}
......
......@@ -83,7 +83,6 @@ describe('AnnotationActionBar', () => {
fakeStore = {
createDraft: sinon.stub(),
getGroup: sinon.stub().returns({}),
isLoggedIn: sinon.stub(),
openSidebarPanel: sinon.stub(),
profile: sinon.stub().returns(fakeUserProfile),
......
......@@ -15,6 +15,7 @@ describe('AnnotationShareControl', () => {
let fakeIsShareableURI;
let fakeShareUri;
let fakeIsIOS;
let fakeStore;
let container;
......@@ -29,7 +30,6 @@ describe('AnnotationShareControl', () => {
<AnnotationShareControl
annotation={fakeAnnotation}
toastMessenger={fakeToastMessenger}
group={fakeGroup}
shareUri={fakeShareUri}
{...props}
/>,
......@@ -80,6 +80,10 @@ describe('AnnotationShareControl', () => {
fakeShareUri = 'https://www.example.com';
fakeIsIOS = sinon.stub().returns(false);
fakeStore = {
getGroup: sinon.stub().returns(fakeGroup),
};
$imports.$mock(mockImportedComponents());
$imports.$mock({
'@hypothesis/frontend-shared': {
......@@ -90,6 +94,7 @@ describe('AnnotationShareControl', () => {
},
'../../util/copy-to-clipboard': fakeCopyToClipboard,
'../../helpers/permissions': { isPrivate: fakeIsPrivate },
'../../store': { useSidebarStore: () => fakeStore },
'../../../shared/user-agent': { isIOS: fakeIsIOS },
});
});
......@@ -99,8 +104,9 @@ describe('AnnotationShareControl', () => {
container.remove();
});
it('does not render component if `group` prop not OK', () => {
const wrapper = createComponent({ group: undefined });
it('does not render component if annotation group is not available', () => {
fakeStore.getGroup.returns(undefined);
const wrapper = createComponent();
assert.equal(wrapper.html(), '');
});
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment