Skip to content

Commit 2ae2eb0

Browse files
david-yz-liuclaude
andcommitted
Allow annotation creation without a prior selection
When grading, the context menu previously disabled all annotation creation options if no region was selected. Graders can now create annotations at any time; a fallback selection is synthesized automatically: - Text/code: first character of the first non-empty line - HTML/Jupyter: first character of the first text node in the iframe - Image: 40×40 px box centred at the right-click position - PDF: 40×40 box centred at the right-click position (with rotation correction), in COORDINATE_MULTIPLIER-scaled percentage space If no fallback can be synthesized (empty file, no text content, etc.) an I18n alert is shown instead of silently failing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 4c0313c commit 2ae2eb0

9 files changed

Lines changed: 859 additions & 21 deletions

File tree

CLAUDE.md

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
# MarkUs – Claude Code notes
2+
3+
## Development environment
4+
5+
All commands run inside Docker:
6+
7+
```bash
8+
docker compose run --rm rails <cmd>
9+
```
10+
11+
Examples:
12+
```bash
13+
docker compose run --rm rails npm install
14+
docker compose run --rm rails node_modules/.bin/webpack --config webpack.production.js
15+
docker compose run --rm rails bundle exec rspec spec/path/to/spec.rb
16+
docker compose run --rm rails npx jest path/to/test
17+
```
18+
19+
Use `node_modules/.bin/webpack` directly rather than `npx webpack` — the latter triggers the CSS watcher via npm scripts and blocks.
20+
21+
## JavaScript assets
22+
23+
- Bundler: **Webpack 5** (`webpack.common.js`, `webpack.development.js`, `webpack.production.js`)
24+
- JS source: `app/javascript/`
25+
- Built output: `app/assets/builds/`
26+
- CSS/SCSS source: `app/assets/stylesheets/`; built by a separate Sass watcher
27+
28+
To verify changes compile (fast — skips minification):
29+
```bash
30+
docker compose run --rm webpack node_modules/.bin/webpack --config webpack.development.js --no-watch
31+
```
32+
33+
To do a one-shot production build (JS + CSS bundled together, slow):
34+
```bash
35+
docker compose run --rm webpack node_modules/.bin/webpack --config webpack.production.js --no-watch
36+
```
37+
38+
**Always use the `webpack` service (not `rails`) and pass `--no-watch`** — using the `rails` service or omitting `--no-watch` enables watch mode and the process never terminates.
39+
40+
## JS tests
41+
42+
```bash
43+
docker compose run --rm rails npx jest # all JS tests
44+
docker compose run --rm rails npx jest path/to/test # specific file
45+
```
46+
47+
## Jest testing conventions
48+
49+
Test files live in `app/javascript/Components/__tests__/` and match `**/__tests__/*.test.[jt]s?(x)` (see `jest.config.js`).
50+
51+
### Importing legacy IIFE scripts
52+
53+
Some JS lives in `app/assets/javascripts/` as plain IIFEs (not ES modules) — e.g. `Annotations/pdf_annotation_manager.js`, `Annotations/globals.js`. To use them in Jest, require them for their side-effects:
54+
55+
```js
56+
// From app/javascript/Components/__tests__/
57+
require("../../../assets/javascripts/Annotations/pdf_annotation_manager");
58+
// Note: 3 levels up reaches app/, then into assets/javascripts/
59+
```
60+
61+
The IIFE assigns to `window.X`, so access the export as `window.PdfAnnotationManager` after requiring.
62+
63+
### Globals defined by legacy scripts
64+
65+
`app/assets/javascripts/Annotations/globals.js` assigns bare globals (`annotation_type`, `ANNOTATION_TYPES`, `annotation_manager`, etc.) to `window`. These are not in the Jest module graph. Set them manually in `beforeEach`:
66+
67+
```js
68+
global.ANNOTATION_TYPES = {CODE: 0, IMAGE: 1, PDF: 2, HTML: 3};
69+
global.annotation_type = global.ANNOTATION_TYPES.PDF;
70+
window.annotation_manager = {getSelection: jest.fn(), ...};
71+
```
72+
73+
`I18n` and `$`/`jQuery` are already available via `jest_env_setup.js`.
74+
75+
### Mocking jQuery plugins (e.g. `$.fn.contextmenu`)
76+
77+
jQuery UI plugins used in production code (like `ui-contextmenu`) are available as npm dependencies but may need mocking for test isolation. Capture the options object by spying before calling the setup function:
78+
79+
```js
80+
let capturedOptions;
81+
jest.spyOn($.fn, "contextmenu").mockImplementation(function(opts) {
82+
if (typeof opts === "object") capturedOptions = opts;
83+
return this;
84+
});
85+
```
86+
87+
Then invoke handlers directly: `capturedOptions.beforeOpen(fakeEvent, fakeUi)`.
88+
89+
## pdfjs-dist
90+
91+
- Imported globally in `app/javascript/application_webpack.js` as `window.pdfjs` and `window.pdfjsViewer`
92+
- Worker bundle is a separate webpack entry (`"pdf.worker": "pdfjs-dist/build/pdf.worker.mjs"`)
93+
- Worker URL wired up at request time in `app/views/layouts/_pdfjs_config.html.erb` via Rails `asset_path`
94+
- MarkUs-specific pdfjs CSS overrides live in `app/assets/stylesheets/common/pdfjs_custom.scss`

app/assets/javascripts/Annotations/html_annotations.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,22 @@ function check_annotation_overlap(range) {
3131
);
3232
}
3333

34-
function get_html_annotation_range() {
34+
function get_html_annotation_range(warn = true) {
3535
const iframe = document.getElementById("html-content");
3636
const target = iframe.contentDocument;
3737
const selection = target.getSelection();
3838
if (selection.rangeCount >= 1) {
3939
const range = selection.getRangeAt(0);
4040
if (check_annotation_overlap(range)) {
4141
alert(I18n.t("results.annotation.no_overlap"));
42-
return {};
42+
return null;
4343
}
4444
if (range.startOffset !== range.endOffset || range.startContainer !== range.endContainer) {
4545
return range;
4646
}
4747
}
48-
alert(I18n.t("results.annotation.select_some_text"));
49-
return {};
48+
if (warn) {
49+
alert(I18n.t("results.annotation.select_some_text"));
50+
}
51+
return null;
5052
}

app/assets/javascripts/Annotations/image_annotation_manager.js

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,41 @@ class ImageAnnotationManager extends AnnotationManager {
313313
return box;
314314
}
315315

316+
/**
317+
* Returns a fallback 40×40 px selection centred at the last right-click position,
318+
* for use when no area is currently selected.
319+
* @returns {{x1, y1, x2, y2}|false}
320+
*/
321+
getFallbackSelection() {
322+
const img = this.image_preview; // the <img> DOM element
323+
if (!img) return false;
324+
325+
const rect = img.getBoundingClientRect();
326+
const displayWidth = rect.width;
327+
const displayHeight = rect.height;
328+
if (displayWidth === 0 || displayHeight === 0) return false;
329+
330+
// Convert page click position to position within the displayed image.
331+
const e = this.last_click_event;
332+
const clickX = e ? e.clientX - rect.left : displayWidth / 2;
333+
const clickY = e ? e.clientY - rect.top : displayHeight / 2;
334+
335+
// Scale from display pixels to image natural pixels.
336+
const scaleX = img.naturalWidth / displayWidth;
337+
const scaleY = img.naturalHeight / displayHeight;
338+
const imgX = Math.round(clickX * scaleX);
339+
const imgY = Math.round(clickY * scaleY);
340+
341+
// 40x40 box centred at click, clamped to image bounds.
342+
const half = 20;
343+
return {
344+
x1: Math.max(0, imgX - half),
345+
y1: Math.max(0, imgY - half),
346+
x2: Math.min(img.naturalWidth, imgX + half),
347+
y2: Math.min(img.naturalHeight, imgY + half),
348+
};
349+
}
350+
316351
get_selection_box_coordinates() {
317352
let img = this.image_preview;
318353
let zoomHeight = img.height;

app/assets/javascripts/Annotations/pdf_annotation_manager.js

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,70 @@
251251
$page.append($control);
252252
};
253253

254+
/**
255+
* Returns a fallback 40×40 selection centred at the last right-click position,
256+
* for use when no area is currently selected.
257+
* Applies the same inverse-rotation correction as getSelection() so that
258+
* renderAnnotation()'s forward rotation produces the correct screen position.
259+
* @returns {{x1, y1, x2, y2, page}|false}
260+
*/
261+
getFallbackSelection() {
262+
// Find which page element was right-clicked.
263+
const e = this.last_click_event;
264+
let pageEl = null;
265+
let pageNumber = 1;
266+
267+
if (e) {
268+
// Walk up from the event target to find the page container.
269+
let el = document.elementFromPoint(e.clientX, e.clientY);
270+
while (el && el !== document.body) {
271+
if (el.dataset && el.dataset.pageNumber) {
272+
pageEl = el;
273+
pageNumber = parseInt(el.dataset.pageNumber, 10);
274+
break;
275+
}
276+
el = el.parentElement;
277+
}
278+
}
279+
280+
if (!pageEl) {
281+
// Fall back to first visible page.
282+
const $firstPage = $(".page[data-page-number]").first();
283+
if ($firstPage.length) {
284+
pageEl = $firstPage[0];
285+
pageNumber = $firstPage.data("page-number");
286+
}
287+
}
288+
289+
if (!pageEl) return false;
290+
291+
const rect = pageEl.getBoundingClientRect();
292+
if (rect.width === 0 || rect.height === 0) return false;
293+
294+
// Click position as percentage of page dimensions, scaled by COORDINATE_MULTIPLIER.
295+
const clickX = e ? e.clientX - rect.left : rect.width / 2;
296+
const clickY = e ? e.clientY - rect.top : rect.height / 2;
297+
const cx = Math.round((clickX / rect.width) * COORDINATE_MULTIPLIER);
298+
const cy = Math.round((clickY / rect.height) * COORDINATE_MULTIPLIER);
299+
300+
// 40px in COORDINATE_MULTIPLIER units relative to page size.
301+
const halfX = Math.round((20 / rect.width) * COORDINATE_MULTIPLIER);
302+
const halfY = Math.round((20 / rect.height) * COORDINATE_MULTIPLIER);
303+
304+
// 40x40 box centred at click, clamped to page bounds.
305+
const box = {
306+
x1: Math.max(0, cx - halfX),
307+
y1: Math.max(0, cy - halfY),
308+
x2: Math.min(COORDINATE_MULTIPLIER, cx + halfX),
309+
y2: Math.min(COORDINATE_MULTIPLIER, cy + halfY),
310+
};
311+
312+
// Apply the same inverse-rotation correction that getSelection() applies,
313+
// so renderAnnotation()'s forward rotation produces the correct screen position.
314+
const rotated = getRotatedCoords(box, 360 - this.angle);
315+
return {...rotated, page: pageNumber};
316+
}
317+
254318
/**
255319
* The following two functions are used to keep track of the orientation of
256320
* the PDF so we know how to render the annotations.

app/assets/javascripts/Annotations/text_annotation_manager.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,27 @@ class TextAnnotationManager extends AnnotationManager {
213213
};
214214
}
215215

216+
/**
217+
* Returns a fallback selection (first character of the first non-empty line)
218+
* for use when no text is currently selected.
219+
* @returns {{line_start, line_end, column_start, column_end}|false}
220+
*/
221+
getFallbackSelection() {
222+
// source_lines[0] is a null dummy; real lines start at index 1.
223+
for (let i = 1; i < this.source_lines.length; i++) {
224+
const lineContent = this.source_lines[i] ? this.source_lines[i].line_node.textContent : "";
225+
if (lineContent.trim().length > 0) {
226+
return {
227+
line_start: i,
228+
line_end: i,
229+
column_start: 0,
230+
column_end: 1,
231+
};
232+
}
233+
}
234+
return false; // empty file or all blank lines
235+
}
236+
216237
// Given some node, traverses upwards until it finds the span element that represents a line of code.
217238
// This is useful for figuring out what text is currently selected, using window.getSelection().anchorNode / focusNode
218239
getRootFromSelection(node) {

app/javascript/Components/Result/context_menu.js

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -136,15 +136,22 @@ export var annotation_context_menu = {
136136
menu_items.download,
137137
],
138138
beforeOpen: function (event, ui) {
139-
// Enable annotation menu items only if a selection has been made
140-
var selection_exists = !!window.annotation_manager.getSelection(false);
141-
$(document).contextmenu("enableEntry", "check_mark_annotation", selection_exists);
142-
$(document).contextmenu("enableEntry", "thumbs_up_annotation", selection_exists);
143-
$(document).contextmenu("enableEntry", "heart_annotation", selection_exists);
144-
$(document).contextmenu("enableEntry", "smile_annotation", selection_exists);
145-
$(document).contextmenu("enableEntry", "new_annotation", selection_exists);
146-
$(document).contextmenu("enableEntry", "common_annotations", selection_exists);
147-
$(document).contextmenu("enableEntry", "copy", selection_exists);
139+
// Store right-click event so annotation managers can use the click position for fallback
140+
// selections. Note: on touch/long-press (taphold: true above), clientX/clientY may be
141+
// zero on the jQuery synthetic event — this is a pre-existing quirk of the taphold path.
142+
if (window.annotation_manager) {
143+
window.annotation_manager.last_click_event = event;
144+
}
145+
// Annotation creation items are always enabled; fallback selection synthesized on use.
146+
$(document).contextmenu("enableEntry", "check_mark_annotation", true);
147+
$(document).contextmenu("enableEntry", "thumbs_up_annotation", true);
148+
$(document).contextmenu("enableEntry", "heart_annotation", true);
149+
$(document).contextmenu("enableEntry", "smile_annotation", true);
150+
$(document).contextmenu("enableEntry", "new_annotation", true);
151+
$(document).contextmenu("enableEntry", "common_annotations", true);
152+
// copy requires an actual browser text selection, not an annotation region.
153+
var text_selected = !!(window.getSelection && window.getSelection().type === "Range");
154+
$(document).contextmenu("enableEntry", "copy", text_selected);
148155

149156
var has_common_annot =
150157
$(document).contextmenu("getMenu").find(".has_common_annotations").length > 0;

app/javascript/Components/Result/result.jsx

Lines changed: 66 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -337,18 +337,31 @@ class Result extends React.Component {
337337
extend_with_selection_data = annotation_data => {
338338
let box;
339339
if (annotation_type === ANNOTATION_TYPES.HTML) {
340-
const range = get_html_annotation_range();
341-
box = {
342-
start_node: pathToNode(range.startContainer),
343-
start_offset: range.startOffset,
344-
end_node: pathToNode(range.endContainer),
345-
end_offset: range.endOffset,
346-
};
340+
const range = get_html_annotation_range(false); // suppress alert
341+
if (range && range.startContainer) {
342+
box = {
343+
start_node: pathToNode(range.startContainer),
344+
start_offset: range.startOffset,
345+
end_node: pathToNode(range.endContainer),
346+
end_offset: range.endOffset,
347+
};
348+
} else {
349+
box = synthesize_html_fallback_selection();
350+
}
347351
} else {
348-
box = window.annotation_manager.getSelection();
352+
// annotation_manager is null only for HTML files, which are handled by the branch
353+
// above. This guard is unreachable in production but kept as a defensive safety net.
354+
if (!window.annotation_manager) return;
355+
box = window.annotation_manager.getSelection(false);
356+
if (!box) {
357+
box = window.annotation_manager.getFallbackSelection();
358+
}
349359
}
350360
if (box) {
351361
return Object.assign(annotation_data, box);
362+
} else {
363+
alert(I18n.t("results.annotation.cannot_annotate_empty"));
364+
return undefined;
352365
}
353366
};
354367

@@ -1091,3 +1104,48 @@ export function makeResult(elem, props) {
10911104
root.render(<Result {...props} ref={component} />);
10921105
return component;
10931106
}
1107+
1108+
/**
1109+
* Synthesize a fallback HTML annotation selection for when no text is currently selected
1110+
* in the HTML iframe. Finds the first text node in the iframe body and creates a
1111+
* single-character range at offset 0.
1112+
*
1113+
* Must live in result.jsx (not html_annotations.js) because it uses pathToNode,
1114+
* which is an ES module export and is not available in the legacy IIFE scripts.
1115+
*
1116+
* @returns {{start_node, start_offset, end_node, end_offset}|null}
1117+
*/
1118+
export function synthesize_html_fallback_selection() {
1119+
const iframe = document.getElementById("html-content");
1120+
if (!iframe) return null;
1121+
const target = iframe.contentDocument;
1122+
if (!target || !target.body) return null;
1123+
1124+
function findFirstTextNode(node) {
1125+
if (node.nodeType === Node.TEXT_NODE && node.nodeValue.trim().length > 0) {
1126+
return node;
1127+
}
1128+
for (const child of node.childNodes) {
1129+
const found = findFirstTextNode(child);
1130+
if (found) return found;
1131+
}
1132+
return null;
1133+
}
1134+
1135+
const textNode = findFirstTextNode(target.body);
1136+
if (!textNode) return null;
1137+
1138+
const range = target.createRange();
1139+
range.setStart(textNode, 0);
1140+
range.setEnd(textNode, 1);
1141+
1142+
if (typeof check_annotation_overlap === "function" && check_annotation_overlap(range))
1143+
return null;
1144+
1145+
return {
1146+
start_node: pathToNode(textNode),
1147+
start_offset: 0,
1148+
end_node: pathToNode(textNode),
1149+
end_offset: 1,
1150+
};
1151+
}

0 commit comments

Comments
 (0)