From 68cd532c25a15d741ca1430251040145a5b68050 Mon Sep 17 00:00:00 2001 From: Gabriel Morador Date: Thu, 24 Sep 2026 13:58:48 -0300 Subject: [PATCH 1/3] EDU role survey: block the sidebar until it is answered While the survey is pending, grey out the sidebar's content and make it unreachable, as the PM confirmed: the panel stays usable, and so does everything the top bar opens. `inert` on `
` and nothing else. It is the only option that also stops Tab reaching the content -- `pointer-events-none` does nothing against the keyboard, and managing `tabIndex` by hand across the editors, menus and links inside would not stay correct between renders -- and it takes the subtree out of the accessibility tree by itself, so no `aria-hidden` on top. Wrapping `
` alone keeps Help, Search and Share, which are its 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. The browserslist targets all ship `inert`, and Preact sets it as a property, so `inert={false}` really does un-inert the element. The root's scroll is taken away while the panel is up, to keep it in place. Help, Search and Share open below the panel and can be clipped on a short sidebar, but each has its own close button, so nobody is stuck. The panel caps its own height, so its buttons stay reachable with large fonts or a narrow sidebar. PendingUpdatesNotification is hidden meanwhile: it is `fixed top-12`, so it would float over the panel's dismiss button, and applying real-time updates to a list nobody can reach has nothing to offer. Existing drafts are deliberately left alone rather than discarded the way logout does: they stay frozen behind the panel and come back once the question is answered. --- src/sidebar/components/HypothesisApp.tsx | 25 +++++-- .../components/InstructorSurveyPanel.tsx | 5 ++ src/sidebar/components/SidebarView.tsx | 35 ++++++---- .../components/test/HypothesisApp-test.js | 68 +++++++++++++++++++ .../components/test/SidebarView-test.js | 20 ++++++ 5 files changed, 136 insertions(+), 17 deletions(-) diff --git a/src/sidebar/components/HypothesisApp.tsx b/src/sidebar/components/HypothesisApp.tsx index 0b3b040c549..4dd8c5e6dc0 100644 --- a/src/sidebar/components/HypothesisApp.tsx +++ b/src/sidebar/components/HypothesisApp.tsx @@ -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)) { @@ -146,12 +148,17 @@ function HypothesisApp({ return (
- {isSidebar && surveyPending && } + {surveyShown && } {route && ( -
+ // `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 `
` 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. +
{route === 'annotation' && } {route === 'notebook' && } {route === 'profile' && } diff --git a/src/sidebar/components/InstructorSurveyPanel.tsx b/src/sidebar/components/InstructorSurveyPanel.tsx index 5e204a30e39..c72dadad49b 100644 --- a/src/sidebar/components/InstructorSurveyPanel.tsx +++ b/src/sidebar/components/InstructorSurveyPanel.tsx @@ -93,6 +93,11 @@ function InstructorSurveyPanel({ diff --git a/src/sidebar/components/SidebarView.tsx b/src/sidebar/components/SidebarView.tsx index 5ec3bec1936..dac0bec8da3 100644 --- a/src/sidebar/components/SidebarView.tsx +++ b/src/sidebar/components/SidebarView.tsx @@ -50,6 +50,7 @@ function SidebarView({ const searchUris = store.searchUris(); const sidebarHasOpened = store.hasSidebarOpened(); + const surveyShown = store.isInstructorSurveyPending(); const userId = store.profile().userid; // If, after loading completes, no `linkedAnnotation` object is present when @@ -130,19 +131,27 @@ function SidebarView({ return (

Annotations

-
- -
+ {/* + Hidden while the EDU role survey is up: this is `fixed top-12`, so it + would float over the survey panel -- greyed out and inert with the rest + of the content, but still on top of the question. Applying real-time + updates to a list nobody can reach has nothing to offer either. + */} + {!surveyShown && ( +
+ +
+ )} {showFilterControls && } {hasDirectLinkedAnnotationError && ( diff --git a/src/sidebar/components/test/HypothesisApp-test.js b/src/sidebar/components/test/HypothesisApp-test.js index a60b191ebad..8bfb81c2a73 100644 --- a/src/sidebar/components/test/HypothesisApp-test.js +++ b/src/sidebar/components/test/HypothesisApp-test.js @@ -219,6 +219,74 @@ describe('HypothesisApp', () => { }); }); + describe('blocking the sidebar behind the survey', () => { + it('makes the content inert and greys it out while the survey is pending', () => { + fakeStore.isInstructorSurveyPending.returns(true); + + const wrapper = createComponent(); + const main = wrapper.find('main'); + + assert.isTrue(main.prop('inert')); + assert.isTrue(main.hasClass('opacity-50')); + }); + + it('leaves the content alone when the survey is not pending', () => { + fakeStore.isInstructorSurveyPending.returns(false); + + const wrapper = createComponent(); + const main = wrapper.find('main'); + + assert.isFalse(main.prop('inert')); + assert.isFalse(main.hasClass('opacity-50')); + }); + + it('takes the scroll away from the root while the survey is pending', () => { + // How the panel is kept in place: the content below it is inert anyway. + fakeStore.isInstructorSurveyPending.returns(true); + + const wrapper = createComponent(); + const root = wrapper.find('[data-testid="hypothesis-app"]'); + + assert.isTrue(root.hasClass('overflow-hidden')); + assert.isFalse(root.hasClass('overflow-auto')); + }); + + it('gives the root its scroll back once the survey is answered', () => { + fakeStore.isInstructorSurveyPending.returns(false); + + const wrapper = createComponent(); + const root = wrapper.find('[data-testid="hypothesis-app"]'); + + assert.isTrue(root.hasClass('overflow-auto')); + assert.isFalse(root.hasClass('overflow-hidden')); + }); + + it('leaves existing drafts alone', () => { + // Deliberately not discardAllDrafts(), which is what logout does: + // destroying someone's unsaved work to collect a survey answer would be + // a bad trade. Drafts stay frozen inside the inert content and come back + // when the question is answered. + fakeStore.isInstructorSurveyPending.returns(true); + fakeStore.countDrafts.returns(2); + + createComponent(); + + assert.notCalled(fakeStore.discardAllDrafts); + assert.equal(fakeStore.countDrafts(), 2); + }); + + ['annotation', 'notebook', 'profile', 'stream'].forEach(route => { + it(`does not block anything outside the sidebar (${route})`, () => { + fakeStore.isInstructorSurveyPending.returns(true); + fakeStore.route.returns(route); + + const wrapper = createComponent(); + + assert.isFalse(wrapper.find('main').prop('inert')); + }); + }); + }); + // Add tests for common behaviors shared between "Log in" and "Sign up" actions. function addCommonLoginTests(action) { const clickButton = wrapper => diff --git a/src/sidebar/components/test/SidebarView-test.js b/src/sidebar/components/test/SidebarView-test.js index ca05f6db7e1..9c456806563 100644 --- a/src/sidebar/components/test/SidebarView-test.js +++ b/src/sidebar/components/test/SidebarView-test.js @@ -53,6 +53,7 @@ describe('SidebarView', () => { hasFetchedProfile: sinon.stub().returns(true), hasSelectedAnnotations: sinon.stub(), hasSidebarOpened: sinon.stub(), + isInstructorSurveyPending: sinon.stub().returns(false), isLoading: sinon.stub().returns(false), isLoggedIn: sinon.stub(), isSidebarPanelOpen: sinon.stub().returns(false), @@ -282,4 +283,23 @@ describe('SidebarView', () => { content: () => createComponent(), }), ); + + describe('the EDU role survey', () => { + it('hides the pending updates notification while the survey is up', () => { + // It is `fixed top-12`, so it would float over the survey panel. + fakeStore.isInstructorSurveyPending.returns(true); + + const wrapper = createComponent(); + + assert.isFalse(wrapper.find('PendingUpdatesNotification').exists()); + }); + + it('shows the pending updates notification otherwise', () => { + fakeStore.isInstructorSurveyPending.returns(false); + + const wrapper = createComponent(); + + assert.isTrue(wrapper.find('PendingUpdatesNotification').exists()); + }); + }); }); From 62f367c5e47da9c35cb951d1431dd444097fedbc Mon Sep 17 00:00:00 2001 From: Gabriel Morador Date: Thu, 24 Sep 2026 13:59:48 -0300 Subject: [PATCH 2/3] EDU role survey: turn annotating into a pointer to the survey With the sidebar blocked, an annotation made on the page has nowhere to go: its draft would open inside the inert content. Instead of creating it, send the user to the survey. The sidebar pushes `setAnnotatingEnabled` to every guest frame -- on connect, for frames that arrive later such as iframes or VitalSource chapters, and on each change. While it is off the adder and the toolbar's buttons still show, but every way of creating an annotation -- the adder, which calls `createAnnotationFromSelection` directly, the toolbar and the keyboard shortcuts, which go through `createAnnotation` -- drops the selection and reports `annotatingBlocked` instead. The sidebar then opens, if it wasn't already, and the survey panel takes focus and plays a short pulse. The pulse is what makes this work with the sidebar already open, where opening it would change nothing on screen and the click would look ignored. It also settles what to do with the sidebar closed: someone who never opens it keeps an adder that leads to the question, rather than a client that seems to have stopped annotating for no visible reason. The store counts the attempts and the panel reacts to each new one, re-keying its card so that a click during the pulse starts it again. The selection is otherwise left alone when annotating is turned off or back on. Clearing it would have reached across frames -- the host answers `textUnselected` by clearing the selection in every other guest -- and text still selected when the survey is answered can be annotated straight away. The blocked path also clears `_pendingKeyboardMode`, which keyboard callers set before calling `createAnnotation` and only clear on rejection, so it can't leak into the next annotation once annotating is back on. As a backstop, FrameSyncService's `createAnnotation` handler refuses an annotation that reaches it while the survey is pending, through the branch it already has for a logged-out user: open the sidebar, delete the annotation in its frame, and never create a draft. --- src/annotator/guest.ts | 48 +++++++- src/annotator/test/guest-test.js | 110 ++++++++++++++++++ .../components/InstructorSurveyPanel.tsx | 25 ++++ .../test/InstructorSurveyPanel-test.js | 92 ++++++++++++++- src/sidebar/services/frame-sync.ts | 45 ++++++- src/sidebar/services/test/frame-sync-test.js | 77 ++++++++++++ src/sidebar/store/modules/session.ts | 29 ++++- .../store/modules/test/session-test.js | 19 +++ src/styles/sidebar/sidebar.css | 15 +++ src/types/port-rpc-calls.d.ts | 20 ++++ 10 files changed, 474 insertions(+), 6 deletions(-) diff --git a/src/annotator/guest.ts b/src/annotator/guest.ts index 149d256827e..a457c25ff04 100644 --- a/src/annotator/guest.ts +++ b/src/annotator/guest.ts @@ -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'; @@ -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; @@ -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 @@ -1041,6 +1060,14 @@ export class Guest * creation. */ async createAnnotation(tool: AnnotationTool): Promise { + 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)) { @@ -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 { + } = {}): Promise { + // The adder's buttons call this directly, not through `createAnnotation`. + if (!this._annotatingEnabled) { + this._reportAnnotatingBlocked(); + return null; + } + const ranges = this.selectedRanges; this.selectedRanges = []; @@ -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(); diff --git a/src/annotator/test/guest-test.js b/src/annotator/test/guest-test.js index 82df0acff40..71922845b18 100644 --- a/src/annotator/test/guest-test.js +++ b/src/annotator/test/guest-test.js @@ -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(); diff --git a/src/sidebar/components/InstructorSurveyPanel.tsx b/src/sidebar/components/InstructorSurveyPanel.tsx index c72dadad49b..147d05306d0 100644 --- a/src/sidebar/components/InstructorSurveyPanel.tsx +++ b/src/sidebar/components/InstructorSurveyPanel.tsx @@ -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; @@ -62,6 +65,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); @@ -98,8 +119,12 @@ function InstructorSurveyPanel({ // 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} > { }; // The panel only counts as seen once the sidebar has been opened; most // tests want it already open. - fakeStore = { hasSidebarOpened: sinon.stub().returns(true) }; + fakeStore = { + hasSidebarOpened: sinon.stub().returns(true), + instructorSurveyNudges: sinon.stub().returns(0), + }; $imports.$mock({ '../store': { useSidebarStore: () => fakeStore }, @@ -68,6 +72,92 @@ describe('InstructorSurveyPanel', () => { ); }); + describe('when the user tries to annotate', () => { + let clock; + + beforeEach(() => { + clock = sinon.useFakeTimers(); + }); + + afterEach(() => { + clock.restore(); + }); + + const card = wrapper => wrapper.find('Card'); + const nudge = (wrapper, count) => { + fakeStore.instructorSurveyNudges.returns(count); + // The store is a fake, so re-render by hand -- with a fresh element, since + // an unchanged one would be skipped -- and use `act` so the effect that + // reacts to the new count runs before asserting. + act(() => { + wrapper.setProps({ + children: , + }); + }); + wrapper.update(); + }; + + it('pulses and takes focus', () => { + const wrapper = createComponent(); + document.activeElement.blur(); + + nudge(wrapper, 1); + + assert.include(card(wrapper).prop('classes'), 'animate-survey-nudge'); + assert.equal( + document.activeElement, + wrapper + .find('section[data-testid="instructor-survey-panel"]') + .getDOMNode(), + ); + }); + + it('stops pulsing once the animation is over', () => { + const wrapper = createComponent(); + nudge(wrapper, 1); + + act(() => { + clock.tick(800); + }); + wrapper.update(); + + assert.notInclude(card(wrapper).prop('classes'), 'animate-survey-nudge'); + }); + + it('pulses again on the next attempt', () => { + const wrapper = createComponent(); + nudge(wrapper, 1); + act(() => { + clock.tick(800); + }); + wrapper.update(); + + nudge(wrapper, 2); + + assert.include(card(wrapper).prop('classes'), 'animate-survey-nudge'); + }); + + it('restarts the pulse on an attempt made while it is playing', () => { + const wrapper = createComponent(); + nudge(wrapper, 1); + const first = card(wrapper).getDOMNode(); + + nudge(wrapper, 2); + + // A fresh element is what makes the browser play the animation again. + assert.notEqual(card(wrapper).getDOMNode(), first); + assert.include(card(wrapper).prop('classes'), 'animate-survey-nudge'); + }); + + it('does not pulse for attempts made before it mounted', () => { + fakeStore.instructorSurveyNudges.returns(3); + + const wrapper = createComponent(); + + assert.notInclude(card(wrapper).prop('classes'), 'animate-survey-nudge'); + }); + }); + it('does not track an impression while the sidebar is closed', () => { // The sidebar iframe is created at page load and starts collapsed, so the // panel mounts without anyone having seen it. diff --git a/src/sidebar/services/frame-sync.ts b/src/sidebar/services/frame-sync.ts index 2c739990faa..db33f307b3a 100644 --- a/src/sidebar/services/frame-sync.ts +++ b/src/sidebar/services/frame-sync.ts @@ -220,6 +220,7 @@ export class FrameSyncService { this._setupSyncToGuests(); this._setupHostEvents(); this._setupFeatureFlagSync(); + this._setupAnnotatingEnabledSync(); this._setupShortcutsSync(); this._setupToastMessengerEvents(); } @@ -425,10 +426,17 @@ export class FrameSyncService { const isLoggedIn = this._store.isLoggedIn(); const hasGroup = this._store.focusedGroup() !== null; - if (!isLoggedIn || !hasGroup) { + // The survey check is belt and braces: the guest should not have got + // this far with annotating off. Reusing this branch means a draft is + // never opened inside the inert sidebar, and the annotation is deleted + // in the frame it came from. + const surveyPending = this._store.isInstructorSurveyPending(); + if (!isLoggedIn || !hasGroup || surveyPending) { this._hostRPC.call('openSidebar'); if (!isLoggedIn) { this._store.openSidebarPanel('loginPrompt'); + } else if (surveyPending) { + this._store.nudgeInstructorSurvey(); } this._guestRPC.forEach(rpc => rpc.call('deleteAnnotation', annot.$tag)); return; @@ -504,6 +512,14 @@ export class FrameSyncService { this._hostRPC.call('openSidebar'); }); + // The user tried to annotate while the survey blocks the sidebar. Open it, + // if it isn't already, and point them at the survey: with the sidebar + // already open, opening it alone would look like the click did nothing. + guestRPC.on('annotatingBlocked', () => { + this._hostRPC.call('openSidebar'); + this._store.nudgeInstructorSurvey(); + }); + guestRPC.on('closeSidebar', () => { this._hostRPC.call('closeSidebar'); }); @@ -512,6 +528,10 @@ export class FrameSyncService { // Synchronize highlight visibility in this guest with the sidebar's controls. guestRPC.call('setHighlightsVisible', this._highlightsVisible); + guestRPC.call( + 'setAnnotatingEnabled', + !this._store.isInstructorSurveyPending(), + ); guestRPC.call('featureFlagsUpdated', this._store.features()); guestRPC.call('shortcutsUpdated', getAllShortcuts()); @@ -568,6 +588,29 @@ export class FrameSyncService { watch(this._store.subscribe, getFlags, sendFlags); } + /** + * Turn annotating off in the guest frames while the sidebar is blocked behind + * the EDU role survey, and back on once it is answered. + * + * Without this the guest would create the annotation and the survey check in + * the `createAnnotation` handler would delete it again, so its highlight + * would flash on the page and vanish. With it the guest creates nothing and + * reports `annotatingBlocked` instead. + * + * The initial value goes out as each guest connects (see `_connectGuest`). + */ + private _setupAnnotatingEnabledSync() { + const getEnabled = () => !this._store.isInstructorSurveyPending(); + + const sendEnabled = (enabled: boolean) => { + for (const guest of this._guestRPC.values()) { + guest.call('setAnnotatingEnabled', enabled); + } + }; + + watch(this._store.subscribe, getEnabled, sendEnabled); + } + /** * Set up synchronization of keyboard shortcuts to guest frames. * diff --git a/src/sidebar/services/test/frame-sync-test.js b/src/sidebar/services/test/frame-sync-test.js index a14933547f5..1d0ec1c8351 100644 --- a/src/sidebar/services/test/frame-sync-test.js +++ b/src/sidebar/services/test/frame-sync-test.js @@ -171,7 +171,9 @@ describe('FrameSyncService', () => { focusedGroup: sinon.stub().returns({ id: 'foobar' }), getFocusFilters: sinon.stub().returns({}), hoverAnnotations: sinon.stub(), + isInstructorSurveyPending: sinon.stub().returns(false), isLoggedIn: sinon.stub().returns(false), + nudgeInstructorSurvey: sinon.stub(), openSidebarPanel: sinon.stub(), selectAnnotations: sinon.stub(), selectTab: sinon.stub(), @@ -1193,6 +1195,81 @@ describe('FrameSyncService', () => { }); }); + describe('turning annotating off behind the EDU role survey', () => { + const setSurveyPending = pending => { + fakeStore.isInstructorSurveyPending.returns(pending); + // Any state change makes the watcher re-read the selector. + fakeStore.setState({}); + }; + + beforeEach(async () => { + await frameSync.connect(); + }); + + it('tells a guest frame when it connects', async () => { + // Guests that connect later -- iframes, VitalSource chapter navigation -- + // need the current value, not just the changes. + setSurveyPending(true); + + await connectGuest(); + + assert.calledWith(guestRPC().call, 'setAnnotatingEnabled', false); + }); + + it('turns annotating off in guests while the survey is pending', async () => { + await connectGuest(); + guestRPC().call.resetHistory(); + + setSurveyPending(true); + + assert.calledWith(guestRPC().call, 'setAnnotatingEnabled', false); + }); + + it('turns annotating back on once the survey is answered', async () => { + await connectGuest(); + setSurveyPending(true); + guestRPC().call.resetHistory(); + + setSurveyPending(false); + + assert.calledWith(guestRPC().call, 'setAnnotatingEnabled', true); + }); + + it('leaves the host frame alone', async () => { + // The toolbar keeps its buttons; the guest turns their use into + // `annotatingBlocked`. + await connectGuest(); + setSurveyPending(true); + + assert.neverCalledWith(hostRPC().call, 'setAnnotatingEnabled'); + }); + + it('opens the sidebar on the survey when a guest reports a blocked annotation', async () => { + await connectGuest(); + + emitGuestEvent('annotatingBlocked'); + + assert.calledWith(hostRPC().call, 'openSidebar'); + assert.calledOnce(fakeStore.nudgeInstructorSurvey); + }); + + it('refuses an annotation that reaches the sidebar anyway', async () => { + // Belt and braces: the guest should not have got this far. No draft is + // opened inside the inert sidebar, the annotation is removed from the + // frame it came from, and the user is pointed at the survey. + await connectGuest(); + fakeStore.isLoggedIn.returns(true); + setSurveyPending(true); + + emitGuestEvent('createAnnotation', { $tag: 't1', target: [] }); + + assert.notCalled(fakeAnnotationsService.create); + assert.calledWith(guestRPC().call, 'deleteAnnotation', 't1'); + assert.calledOnce(fakeStore.nudgeInstructorSurvey); + assert.notCalled(fakeStore.openSidebarPanel); + }); + }); + describe('sending shortcut updates to frames', () => { beforeEach(async () => { await frameSync.connect(); diff --git a/src/sidebar/store/modules/session.ts b/src/sidebar/store/modules/session.ts index 887040b6a92..aebe8934200 100644 --- a/src/sidebar/store/modules/session.ts +++ b/src/sidebar/store/modules/session.ts @@ -30,6 +30,13 @@ export type State = { * Profile object fetched from the `/api/profile` endpoint. */ profile: Profile; + + /** + * How many times the user has been pointed at the EDU role survey because + * they tried to annotate while it blocks the sidebar. Only changes matter: + * the survey panel draws attention to itself on each one. + */ + instructorSurveyNudges: number; }; /** @@ -52,6 +59,7 @@ function initialState(settings: SidebarSettings): State { defaultAuthority: settings?.authDomain ?? '', features: settings.features ?? [], profile: initialProfile, + instructorSurveyNudges: 0, }; } @@ -61,6 +69,10 @@ const reducers = { profile: { ...action.profile }, }; }, + + NUDGE_INSTRUCTOR_SURVEY(state: State) { + return { instructorSurveyNudges: state.instructorSurveyNudges + 1 }; + }, }; /** @@ -70,6 +82,13 @@ function updateProfile(profile: Profile) { return makeAction(reducers, 'UPDATE_PROFILE', { profile }); } +/** + * Point the user at the EDU role survey: they tried to do something it blocks. + */ +function nudgeInstructorSurvey() { + return makeAction(reducers, 'NUDGE_INSTRUCTOR_SURVEY', undefined); +} + function defaultAuthority(state: State) { return state.defaultAuthority; } @@ -111,13 +130,17 @@ function isFeatureEnabled(state: State, feature: string) { * Return true if the EDU role survey is being asked of this user. * * The single source of truth for the survey's visibility, shared by the panel - * and (in the sidebar-blocking work) by FrameSyncService, so that the two can't - * disagree about whether the survey is up. + * and by FrameSyncService, so that the two can't disagree about whether the + * survey is up. */ function isInstructorSurveyPending(state: State) { return shouldShowInstructorSurvey(state.profile, features(state)); } +function instructorSurveyNudges(state: State) { + return state.instructorSurveyNudges; +} + /** * Return true if the user's profile has been fetched. This can be used to * distinguish the dummy profile returned by `profile()` on startup from a @@ -147,6 +170,7 @@ export const sessionModule = createStoreModule(initialState, { reducers, actionCreators: { + nudgeInstructorSurvey, updateProfile, }, @@ -155,6 +179,7 @@ export const sessionModule = createStoreModule(initialState, { features, hasFetchedProfile, isFeatureEnabled, + instructorSurveyNudges, isInstructorSurveyPending, isLoggedIn, profile, diff --git a/src/sidebar/store/modules/test/session-test.js b/src/sidebar/store/modules/test/session-test.js index c33b35ce24d..6fe311921df 100644 --- a/src/sidebar/store/modules/test/session-test.js +++ b/src/sidebar/store/modules/test/session-test.js @@ -149,4 +149,23 @@ describe('sidebar/store/modules/session', () => { assert.isFalse(store.isInstructorSurveyPending()); }); }); + + describe('#nudgeInstructorSurvey', () => { + it('counts each attempt to get past the survey', () => { + assert.equal(store.instructorSurveyNudges(), 0); + + store.nudgeInstructorSurvey(); + store.nudgeInstructorSurvey(); + + assert.equal(store.instructorSurveyNudges(), 2); + }); + + it('leaves the profile alone', () => { + store.updateProfile({ userid: 'john', features: {}, preferences: {} }); + + store.nudgeInstructorSurvey(); + + assert.equal(store.profile().userid, 'john'); + }); + }); }); diff --git a/src/styles/sidebar/sidebar.css b/src/styles/sidebar/sidebar.css index 2fd19cca709..9b0f1763728 100644 --- a/src/styles/sidebar/sidebar.css +++ b/src/styles/sidebar/sidebar.css @@ -26,6 +26,21 @@ } } + /* Points the user at the EDU role survey. Keep in step with + NUDGE_DURATION_MS in InstructorSurveyPanel. */ + --animate-survey-nudge: survey-nudge 0.8s ease-in-out; + @keyframes survey-nudge { + 0%, + 100% { + transform: scale(1); + box-shadow: 0 0 0 0 transparent; + } + 30% { + transform: scale(1.03); + box-shadow: 0 0 0 3px var(--color-brand); + } + } + --animate-updates-notification-slide-in: updates-notification-slide-in 0.3s ease-in; @keyframes updates-notification-slide-in { diff --git a/src/types/port-rpc-calls.d.ts b/src/types/port-rpc-calls.d.ts index 35dd84c9857..7c085b4ac2f 100644 --- a/src/types/port-rpc-calls.d.ts +++ b/src/types/port-rpc-calls.d.ts @@ -115,6 +115,13 @@ export type GuestToSidebarCalls = CommonCalls & { /** Request to open the sidebar. */ openSidebar(): void; + /** + * The user tried to create an annotation while annotating is turned off + * (see `setAnnotatingEnabled`). Nothing was created; the sidebar opens and + * points the user at what is blocking it. + */ + annotatingBlocked(): void; + /** The URIs or metadata of the document in the guest frame changed. */ documentInfoChanged(info: DocumentInfo): void; @@ -231,6 +238,19 @@ export type SidebarToGuestCalls = { /** Scroll an annotation into view. */ scrollToAnnotation(tag: string): void; + /** + * Turn annotating in the guest frames on or off. + * + * Pushed from the sidebar rather than derived from a feature flag on the + * annotator side, because what it reflects -- the EDU role survey blocking + * the sidebar -- is sidebar state that the annotator has no other way to + * know about. + * + * While it is off the adder and the toolbar's buttons still show, but using + * them reports `annotatingBlocked` instead of creating an annotation. + */ + setAnnotatingEnabled(enabled: boolean): void; + setHighlightsVisible(visible: boolean): void; /** From 89014f43a80f59809cdffde0037074978a28ac14 Mon Sep 17 00:00:00 2001 From: Gabriel Morador Date: Tue, 29 Sep 2026 11:30:33 -0300 Subject: [PATCH 3/3] EDU role survey: the question's copy --- .../components/InstructorSurveyPanel.tsx | 13 +++++++++--- .../test/InstructorSurveyPanel-test.js | 21 ++++++++++++++++++- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/sidebar/components/InstructorSurveyPanel.tsx b/src/sidebar/components/InstructorSurveyPanel.tsx index 147d05306d0..6ae777a083c 100644 --- a/src/sidebar/components/InstructorSurveyPanel.tsx +++ b/src/sidebar/components/InstructorSurveyPanel.tsx @@ -32,6 +32,7 @@ function InstructorSurveyPanel({ }: InstructorSurveyPanelProps) { const store = useSidebarStore(); const headingId = useId(); + const introId = useId(); const container = useRef(null); const [submitting, setSubmitting] = useState(false); @@ -103,6 +104,7 @@ function InstructorSurveyPanel({ return (
-

- Are you a course instructor? -

+
+

+ Hello! We have a quick question before your next annotation. +

+

+ Are you a course instructor using Hypothesis? +

+