Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 46 additions & 2 deletions src/annotator/guest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,17 @@ export class Guest
private _outsideAssignmentNotice: OutsideAssignmentNoticeController | null;
private _commentsMode: boolean;

/**
* Whether new annotations can be created in this frame.
*
* Pushed from the sidebar, which turns it off while it is blocked behind the
* EDU role survey. The adder and the toolbar keep working; what changes is
* that creating an annotation asks the sidebar to show what is blocking it
* instead. Defaults to true so that a guest that connects before the sidebar
* has told it anything behaves as it always has.
*/
private _annotatingEnabled: boolean;

/** Pending keyboard mode to activate when annotation mode starts */
private _pendingKeyboardMode?: 'move' | 'resize';

Expand All @@ -295,6 +306,7 @@ export class Guest
this.element = element;
this._contentReady = config.contentReady;
this._commentsMode = config.commentsMode ?? false;
this._annotatingEnabled = true;
this._hostFrame = hostFrame;
this._highlightsVisible = false;
this._isAdderVisible = false;
Expand Down Expand Up @@ -679,6 +691,13 @@ export class Guest
this.setHighlightsVisible(showHighlights, false /* notifyHost */);
});

this._sidebarRPC.on('setAnnotatingEnabled', (enabled: boolean) => {
// Nothing else to do: the selection and the adder are left alone either
// way, so text still selected when the survey is answered can be
// annotated straight away.
this._annotatingEnabled = enabled;
});

this._sidebarRPC.on('deleteAnnotation', (tag: string) => this.detach(tag));

// Expose document info to the sidebar on demand, so that annotation
Expand Down Expand Up @@ -1041,6 +1060,14 @@ export class Guest
* creation.
*/
async createAnnotation(tool: AnnotationTool): Promise<AnnotationData | null> {
if (!this._annotatingEnabled) {
// Callers that start a keyboard draw set the mode before calling this,
// and only clear it on rejection. Drop it here, or it would carry into
// the next annotation once annotating is back on.
this._pendingKeyboardMode = undefined;
this._reportAnnotatingBlocked();
return null;
}
if (tool === 'selection') {
return this.createAnnotationFromSelection();
} else if (['rect', 'point'].includes(tool)) {
Expand Down Expand Up @@ -1120,11 +1147,17 @@ export class Guest
* @param [options.highlight] - If true, the new annotation has
* the `$highlight` flag set, causing it to be saved immediately without
* prompting for a comment.
* @return The new annotation
* @return The new annotation, or `null` if annotating is turned off
*/
async createAnnotationFromSelection({
highlight = false,
} = {}): Promise<AnnotationData> {
} = {}): Promise<AnnotationData | null> {
// The adder's buttons call this directly, not through `createAnnotation`.
if (!this._annotatingEnabled) {
this._reportAnnotatingBlocked();
return null;
}

const ranges = this.selectedRanges;
this.selectedRanges = [];

Expand Down Expand Up @@ -1210,6 +1243,17 @@ export class Guest
this._adder.show(focusRect, isBackwards);
}

/**
* Tell the sidebar that an annotation was refused because annotating is off,
* so that it can open and show why. The selection is dropped as it would be
* after a successful annotation, which also hides the adder.
*/
private _reportAnnotatingBlocked() {
this.selectedRanges = [];
removeTextSelection();
this._sidebarRPC.call('annotatingBlocked');
}

_onClearSelection() {
this._isAdderVisible = false;
this._adder.hide();
Expand Down
110 changes: 110 additions & 0 deletions src/annotator/test/guest-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1448,6 +1448,116 @@ describe('Guest', () => {
assert.calledWith(hostRPC().call, 'textSelected');
});

context('when annotating is disabled', () => {
it('still shows the adder on a selection', () => {
// So that using it can lead the user to what is blocking it.
createGuest();
emitSidebarEvent('setAnnotatingEnabled', false);

simulateSelectionWithText();

assert.called(FakeAdder.instance.show);
});

it('leaves an existing selection alone when it is turned off', () => {
createGuest();
simulateSelectionWithText();
FakeAdder.instance.hide.resetHistory();
hostRPC().call.resetHistory();

emitSidebarEvent('setAnnotatingEnabled', false);

assert.notCalled(FakeAdder.instance.hide);
assert.neverCalledWith(hostRPC().call, 'textUnselected');
});

it('reports the attempt instead of annotating from the adder', async () => {
const guest = createGuest();
emitSidebarEvent('setAnnotatingEnabled', false);
simulateSelectionWithText();
sidebarRPC().call.resetHistory();

const annotation = await guest.createAnnotationFromSelection();

assert.isNull(annotation);
assert.calledWith(sidebarRPC().call, 'annotatingBlocked');
assert.neverCalledWith(sidebarRPC().call, 'createAnnotation');
assert.equal(guest.selectedRanges.length, 0);
});

it('reports the attempt instead of creating a shape annotation', async () => {
const guest = createGuest();
emitSidebarEvent('setAnnotatingEnabled', false);

assert.isNull(await guest.createAnnotation('rect'));
assert.calledWith(sidebarRPC().call, 'annotatingBlocked');
});

it('reports the attempt when the toolbar asks for an annotation', () => {
createGuest();
emitSidebarEvent('setAnnotatingEnabled', false);

emitHostEvent('createAnnotation', { tool: 'rect' });

assert.calledWith(sidebarRPC().call, 'annotatingBlocked');
});

it('does not carry a keyboard mode into the next annotation', async () => {
const guest = createGuest();
fakeDrawTool.getKeyboardModeState.returns({ keyboardActive: false });
emitSidebarEvent('setAnnotatingEnabled', false);

emitHostEvent('activateMoveMode');
await delay(0);

assert.isUndefined(guest._pendingKeyboardMode);
assert.notCalled(fakeDrawTool.draw);
assert.calledWith(sidebarRPC().call, 'annotatingBlocked');
});

it('reports the attempt when a keyboard shortcut asks for an annotation', async () => {
fakeIntegration.supportedTools.returns(['rect']);
fakeDrawTool.getKeyboardModeState.returns({ keyboardActive: false });
createGuest();
emitSidebarEvent('featureFlagsUpdated', { vpat_keyboard: true });
emitSidebarEvent('setAnnotatingEnabled', false);

document.body.dispatchEvent(
new KeyboardEvent('keydown', {
ctrlKey: true,
shiftKey: true,
key: 'y',
bubbles: true,
}),
);
await delay(0);

assert.notCalled(fakeDrawTool.draw);
assert.calledWith(sidebarRPC().call, 'annotatingBlocked');
});

it('does not report cancelling a drawing tool', () => {
createGuest();
emitSidebarEvent('setAnnotatingEnabled', false);

emitHostEvent('createAnnotation', { tool: null });

assert.neverCalledWith(sidebarRPC().call, 'annotatingBlocked');
});

it('annotates again once it is turned back on', async () => {
const guest = createGuest();
emitSidebarEvent('setAnnotatingEnabled', false);
emitSidebarEvent('setAnnotatingEnabled', true);
simulateSelectionWithText();

const annotation = await guest.createAnnotationFromSelection();

assert.isNotNull(annotation);
assert.calledWith(sidebarRPC().call, 'createAnnotation', annotation);
});
});

it('calls "textUnselected" RPC method when clearing text selection', () => {
createGuest();

Expand Down
25 changes: 21 additions & 4 deletions src/sidebar/components/HypothesisApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ function HypothesisApp({
const isThemeClean = settings.theme === 'clean';

const isSidebar = route === 'sidebar';
const surveyPending = store.isInstructorSurveyPending();
// One state drives the survey panel, the blocking of the content below it and
// the scroll lock that keeps the panel in place.
const surveyShown = isSidebar && store.isInstructorSurveyPending();

useEffect(() => {
if (shouldAutoDisplayTutorial(isSidebar, profile, settings)) {
Expand Down Expand Up @@ -146,12 +148,17 @@ function HypothesisApp({
return (
<div
className={classnames(
'h-full min-h-full overflow-auto',
'h-full min-h-full',
// Precise padding to align with annotation cards in content
// Larger padding on bottom for wide screens
'lg:pb-16 bg-grey-2',
'js-thread-list-scroll-root',
{
// Pin the survey panel by taking the scroll away from the root.
// Help, Search and Share open below it and can be clipped on a short
// sidebar, but each has its own close button, so nobody is stuck.
'overflow-auto': !surveyShown,
'overflow-hidden': surveyShown,
'theme-clean': isThemeClean,
// Make room at top for the TopBar (40px) plus custom padding (9px)
// but not in the Notebook or Profile, which don't use the TopBar
Expand All @@ -175,13 +182,23 @@ function HypothesisApp({
)}
<div className="container">
<ToastMessages />
{isSidebar && surveyPending && <InstructorSurveyPanel />}
{surveyShown && <InstructorSurveyPanel />}
<HelpPanel />
<SearchPanel />
<SharePanel shareTab={!isThirdParty} />

{route && (
<main>
// `inert` rather than `pointer-events-none` plus `aria-hidden`: it is
// the only one of the three that also stops Tab reaching the content,
// and it takes the subtree out of the accessibility tree by itself.
// Wrapping `<main>` and nothing else keeps Help, Search and Share --
// siblings inside `.container` -- usable from the top bar, and keeps
// ToastMessages outside, so the error toast from a failed answer can
// still be read and dismissed.
<main
className={classnames({ 'opacity-50': surveyShown })}
inert={surveyShown}
>
{route === 'annotation' && <AnnotationView onLogin={login} />}
{route === 'notebook' && <NotebookView />}
{route === 'profile' && <ProfileView />}
Expand Down
43 changes: 40 additions & 3 deletions src/sidebar/components/InstructorSurveyPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import type { AnalyticsService } from '../services/analytics';
import type { SessionService } from '../services/session';
import { useSidebarStore } from '../store';

/** Matches the length of the `survey-nudge` animation in sidebar.css. */
const NUDGE_DURATION_MS = 800;

export type InstructorSurveyPanelProps = {
// Injected
analytics: AnalyticsService;
Expand All @@ -29,6 +32,7 @@ function InstructorSurveyPanel({
}: InstructorSurveyPanelProps) {
const store = useSidebarStore();
const headingId = useId();
const introId = useId();
const container = useRef<HTMLElement | null>(null);
const [submitting, setSubmitting] = useState(false);

Expand Down Expand Up @@ -62,6 +66,24 @@ function InstructorSurveyPanel({
}
}, [sidebarHasOpened]);

// Draw attention to the panel when the user tries to annotate while it
// blocks the sidebar: focus it and play a short pulse, so the click visibly
// leads here -- the sidebar may already have been open, in which case nothing
// else on screen would change. Only changes count, not the value it mounts
// with.
const nudges = store.instructorSurveyNudges();
const initialNudges = useRef(nudges);
const [nudging, setNudging] = useState(false);
useEffect(() => {
if (nudges === initialNudges.current) {
return () => {};
}
container.current?.focus();
setNudging(true);
const timeout = setTimeout(() => setNudging(false), NUDGE_DURATION_MS);
return () => clearTimeout(timeout);
}, [nudges]);

const submit = useCallback(
async (response: InstructorSurveyResponse) => {
setSubmitting(true);
Expand All @@ -82,6 +104,7 @@ function InstructorSurveyPanel({

return (
<section
aria-describedby={introId}
aria-labelledby={headingId}
// Same outer shape as the Help, Search and Share panels (SidebarPanel),
// so it lines up with whichever of them is open below it.
Expand All @@ -93,8 +116,17 @@ function InstructorSurveyPanel({
<Card
classes={classnames(
'relative flex flex-col gap-3 p-3 text-color-text text-sm',
// The root's scroll is taken away while this is up, so a panel
// taller than the viewport -- large fonts, a narrow sidebar -- would
// put its own buttons out of reach. Cheap insurance; the copy is
// short.
'max-h-[calc(100dvh-49px)] overflow-y-auto',
{ 'animate-survey-nudge': nudging },
)}
data-testid="instructor-survey-card"
// A new card per attempt, so that a click while the pulse is still
// playing starts it again instead of leaving the class in place.
key={nudges}
>
<CloseButton
classes={classnames(
Expand All @@ -111,9 +143,14 @@ function InstructorSurveyPanel({
title="Dismiss survey"
variant="custom"
/>
<h2 className="m-0 text-base font-medium" id={headingId}>
Are you a course instructor?
</h2>
<div className="flex flex-col gap-1 pr-6">
<p className="m-0" id={introId}>
Hello! We have a quick question before your next annotation.
</p>
<h2 className="m-0 text-base font-medium" id={headingId}>
Are you a course instructor using Hypothesis?
</h2>
</div>
<div className="flex flex-row gap-2">
<Button
data-testid="instructor-survey-yes"
Expand Down
Loading
Loading