Skip to content
Merged
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
39 changes: 39 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,45 @@ The release run heads these entries with the version and opens a fresh

## Unreleased

- `odr.editing` is on every document view, not only on a sheet's. The mode,
the refusals, the log and the `odr.onEdit*` callbacks are one surface a host
wires per document, and each format attaches its own editor to it.

- **Breaking**: `HtmlConfig::editable` writes the editing scaffolding rather
than `contenteditable` - the address an op names, the lock on a locked cell,
the state on `<body>`, the editor script. `odr.editing.enable()` writes
`contenteditable`, so switching modes needs no second render.

- **Breaking**: a render with `editable` off carries no editing markup at all,
and the document's editable state moved off the `.odr-sheet` table onto
`<body>` as `data-odr-editable`. The table keeps `data-odr-sheet`.

- **Breaking**: a refused new line reaches `odr.onEditRefused` with reason
`newLine` rather than `odr.onError`, keeps code 1, and now fires only inside
an editable run while the mode is on.

- `HtmlConfig::keyboard_navigation` and `keyboard_shortcuts`, both on by
default, decide whether the page takes the keys that move the selection and
the undo chord. An open editor's own keys are never taken away.

- A text document is edited as a document: the mode makes the whole view
editable rather than each run, so the caret, a selection and a double click
cross runs and paragraphs the way a reader expects.

- Every edit a text document cannot replay is refused through
`odr.onEditRefused` rather than silently impossible: a new line
(`newLine`, 1), an edit spanning two runs or landing outside every run
(`range`, 8), and anything else the browser offers (`unsupportedEdit`, 7).

- **Fix**: searching a text document while the mode is on no longer marks it
unsaved. The log is collected from `input`, which the browser raises for an
edit it applied, rather than from every text mutation - a search highlighting
nine matches was nine no-op `setText` operations.

- **Fix**: double-clicking a sheet cell no longer flashes its border off. A
click on the pinned cell clears the pin, and the second click of a double
click was taking it - so selecting a word left the border coming and going.

- A cell of several runs is written rather than locked: the write replaces what
the cell shows with one run. A cell holding one run is written through it, so
that run keeps its style. The `rich` lock stays on what a write would take
Expand Down
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ set(ODR_FRONTEND_ASSETS
"search.css"
"search-dark.css"
"document.js"
"editing.js"
"search.js"
"spreadsheet.js"
"sheet-editing.js"
Expand Down
4 changes: 4 additions & 0 deletions apple/include/OdrCoreObjC/ODRHtml.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ NS_SWIFT_NAME(HtmlConfig)
@property(nonatomic) BOOL relativeResourcePaths;

@property(nonatomic) BOOL editable;
/// Whether the view's scripts take the keys that move the selection.
@property(nonatomic) BOOL keyboardNavigation;
/// Whether the view's scripts take the editing chords: undo and redo.
@property(nonatomic) BOOL keyboardShortcuts;
@property(nonatomic) BOOL textDocumentMargin;

@property(nonatomic) ODRHtmlColorScheme colorScheme;
Expand Down
4 changes: 4 additions & 0 deletions apple/src/ODRHtml.mm
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ - (instancetype)initWithNativeConfig:(const odr::HtmlConfig &)config {
_resourcePath = to_nsstring(config.resource_path);
_relativeResourcePaths = config.relative_resource_paths ? YES : NO;
_editable = config.editable ? YES : NO;
_keyboardNavigation = config.keyboard_navigation ? YES : NO;
_keyboardShortcuts = config.keyboard_shortcuts ? YES : NO;
_textDocumentMargin = config.text_document_margin ? YES : NO;
_colorScheme = static_cast<ODRHtmlColorScheme>(config.color_scheme);
if (config.spreadsheet_limit.has_value()) {
Expand Down Expand Up @@ -156,6 +158,8 @@ - (instancetype)initWithNativeConfig:(const odr::HtmlConfig &)config {
}
config.relative_resource_paths = _relativeResourcePaths == YES;
config.editable = _editable == YES;
config.keyboard_navigation = _keyboardNavigation == YES;
config.keyboard_shortcuts = _keyboardShortcuts == YES;
config.text_document_margin = _textDocumentMargin == YES;
config.color_scheme = static_cast<odr::HtmlColorScheme>(_colorScheme);
if (_spreadsheetLimit != nil) {
Expand Down
301 changes: 285 additions & 16 deletions docs/design/editing.md

Large diffs are not rendered by default.

45 changes: 36 additions & 9 deletions docs/design/spreadsheet-editing.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,9 @@ results go stale the moment an input changes.
| Number formats | β€” | Not parsed in either engine. ODS shows the producer's cached `text:p`; XLSX shows the raw `<v>` (a date is its serial) |
| Formulas | `sheet_cell_value` | The expression is read and handed out as a string (step 0.1, landed); nothing parses or evaluates it. XLSX shows the cached `<v>`, ODS the cached `text:p`. `xls` and `numbers` drop the expression at parse time |
| Browser: sheet script | `html/frontend/spreadsheet.js` | Hover/pin, raise a clipped cell over its neighbours, sort rows in the DOM. Sorting reorders `<tr>`s, so a row's identity is its `<th>` label, not its index. Publishes `odr.sheet` (step 1.1, landed), and the value and reflow half of it (steps 1.2/1.3, landed) |
| Browser: editing script | `html/frontend/document.js` | A `MutationObserver` over `contenteditable` runs keyed by `data-odr-path`; `odr.generateDiff()` emits the envelope |
| Browser: sheet editor | `html/frontend/sheet-editing.js` | `odr.editing` with the mode, the locks and the refusals (step 1.1, landed), and the overlay that types into a cell (steps 1.2/1.3, landed). Undo/redo and `committed()` are step 1.4 |
| Browser: the mode | `html/frontend/editing.js` | `odr.editing` β€” the mode, the refusal table, the log a save reads and the `odr.on*` callbacks, generic over every format. An editor attaches to it ([`editing.md`](editing.md) decision 9, step 1.6, landed) |
| Browser: text editor | `html/frontend/document.js` | The skeleton, attached to the mode: `contenteditable` runs keyed by `data-odr-path`, a `MutationObserver`, one `setText` op per changed run. No undo |
| Browser: sheet editor | `html/frontend/sheet-editing.js` | The cell overlay, the locks and the position map (steps 1.1 to 1.4, landed), attached to the mode as one editor |
| Wire format | `document.cpp::Document::edit` | The op envelope, `setCell` and `setText` (step 0.4, landed) |
| Addressing | `DocumentPath` | Already spells a cell by position: `/child:0/cell:A1/...` |
| Capabilities | `file_type_table.cpp` | `ods` and `xlsx` declare `edit` and `save` (step 0.2, landed); `csv` declares neither. `odr_test` checks the declaration against `Document::is_editable` |
Expand Down Expand Up @@ -104,15 +105,15 @@ any decode of the same file.

### 3. Editing is a browser mode, not markup

`odr.editing.enable()` / `disable()` turns the mode on; `HtmlConfig::editable`
stops changing what a sheet writes. The page carries only what the browser
cannot work out for itself:
`odr.editing.enable()` / `disable()` turns the mode on, and switching it changes
nothing a sheet writes β€” the user never translates twice for it. The page
carries only what the browser cannot work out for itself:

- a **lock** on a cell that cannot be edited, as a class plus its reason β€”
`formula`, `rich` (several paragraphs, a link, a line break), `shapes` only
where the cell is nothing but its anchored drawings;
- whether the **document** can be edited at all, one attribute on the table,
so `enable()` can refuse with a reason before the user clicks anything.
- whether the **document** can be edited at all, so `enable()` can refuse with
a reason before the user clicks anything.

Everything else β€” including every empty cell β€” is editable. The cost is a
class on the locked cells only, nothing on the half million others.
Expand All @@ -128,6 +129,24 @@ read-only document, outlines it briefly and calls `odr.onEditRefused` so the
host can say why β€” a snackbar on mobile. A silent no-op is the frustrating
outcome the mode exists to avoid. Decision 7 is the channel.

**The mode itself is generic, and it moved.** This decision was written when the
sheet was the only editor, so `sheet-editing.js` held the mode, the refusal
table and the callbacks. Every format wants those, so they are
[`editing.md`](editing.md) decisions 9 to 12 now, and `frontend/editing.js`
holds them:

- the sheet script **attaches** an editor to `odr.editing` and states no mode of
its own;
- the document's editable state is one attribute on `<body>`, not on the
`.odr-sheet` table (decision 10) β€” which answers the open question below;
- `HtmlConfig::editable` writes the scaffolding, the lock classes included, and
a read-only render carries none of it (decision 11);
- the arrow keys and the undo chord are configurable, because the sheet takes
them in the capture phase and a host may need them (decision 12).

What stays here is the sheet's own: the cell overlay, the locks and their
reasons, the position map, and the `setCell` op.

### 4. The type follows the content

The typed string is parsed by a strict grammar: optional sign, digits, one `.`,
Expand Down Expand Up @@ -379,6 +398,13 @@ Each step ships on its own. "Both" means `.ods` and `.xlsx`.
wasm example is the host-wiring reference for droid/ios. A view holds its own
log, so the example writes it into the document when the view goes away as
well as on save.
6. **Landed.** The mode is generic. `frontend/editing.js` owns `odr.editing`
and every format's editor attaches to it; the document's editable state moved
to `<body>`; `HtmlConfig::editable` writes the scaffolding and a read-only
render carries none of it; `keyboard_navigation` and `keyboard_shortcuts`
let a host keep the arrows and the undo chord. See
[`editing.md`](editing.md) decisions 9 to 12 β€” a `.docx` view has the same
`odr.editing` a sheet does, with the text skeleton behind it.

### Step 2 β€” Materialise the cells that are not there

Expand Down Expand Up @@ -536,7 +562,8 @@ Ordered by value over cost; all in step 0 or 1.
the compromise for step 1.
- Where does the document locale come from for the decimal separator β€”
`settings.xml`, the number format, the host?
- Does the read-only document attribute belong on the table or in a
page-level `data-odr-*` block the text editor will want too?
- **Answered** ([`editing.md`](editing.md) decision 10): the page-level block,
as `data-odr-editable` on `<body>`. The frame is a fact about the document,
and a `.docx` view has no table to hang it on.
- Should the refusal codes be generated from one C++ table so the bindings can
hand a host the same list, rather than living only in the emitted script?
5 changes: 5 additions & 0 deletions jni/java/app/opendocument/core/HtmlConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ public final class HtmlConfig {

public boolean editable = false;

/** Whether the view's scripts take the keys that move the selection. */
public boolean keyboardNavigation = true;
/** Whether the view's scripts take the editing chords: undo and redo. */
public boolean keyboardShortcuts = true;

public boolean textDocumentMargin = false;

/** The colors a document renders against. */
Expand Down
4 changes: 4 additions & 0 deletions jni/src/jni_style.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,8 @@ jobject html_config_to_java(JNIEnv *env, const odr::HtmlConfig &config) {
set_string("resourcePath", config.resource_path);
set_boolean("relativeResourcePaths", config.relative_resource_paths);
set_boolean("editable", config.editable);
set_boolean("keyboardNavigation", config.keyboard_navigation);
set_boolean("keyboardShortcuts", config.keyboard_shortcuts);
set_boolean("textDocumentMargin", config.text_document_margin);
set_object("colorScheme", "Lapp/opendocument/core/HtmlColorScheme;",
enum_from_code(env, "app/opendocument/core/HtmlColorScheme",
Expand Down Expand Up @@ -573,6 +575,8 @@ odr::HtmlConfig html_config_from_java(JNIEnv *env, jobject config) {
}
result.relative_resource_paths = get_boolean("relativeResourcePaths");
result.editable = get_boolean("editable");
result.keyboard_navigation = get_boolean("keyboardNavigation");
result.keyboard_shortcuts = get_boolean("keyboardShortcuts");
result.text_document_margin = get_boolean("textDocumentMargin");
{
const jint code = enum_ordinal(
Expand Down
3 changes: 3 additions & 0 deletions python/src/bind_html.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ void odr_python::bind_html(py::module_ &m) {
.def_readwrite("relative_resource_paths",
&odr::HtmlConfig::relative_resource_paths)
.def_readwrite("editable", &odr::HtmlConfig::editable)
.def_readwrite("keyboard_navigation",
&odr::HtmlConfig::keyboard_navigation)
.def_readwrite("keyboard_shortcuts", &odr::HtmlConfig::keyboard_shortcuts)
.def_readwrite("text_document_margin",
&odr::HtmlConfig::text_document_margin)
.def_readwrite("color_scheme", &odr::HtmlConfig::color_scheme)
Expand Down
4 changes: 4 additions & 0 deletions python/tests/test_html.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,16 @@ def test_html_config_defaults():
config = pyodr.HtmlConfig()
assert config.embed_images
assert not config.editable
assert config.keyboard_navigation
assert config.keyboard_shortcuts
assert config.spreadsheet_gridlines == pyodr.HtmlTableGridlines.soft

config.editable = True
config.keyboard_navigation = False
config.format_html = True
config.spreadsheet_limit = pyodr.TableDimensions(100, 100)
assert config.editable
assert not config.keyboard_navigation
assert config.spreadsheet_limit.rows == 100

assert config.spreadsheet_cell_limit == 500000
Expand Down
10 changes: 9 additions & 1 deletion src/odr/html.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,17 @@ struct HtmlConfig {
/// output stays movable.
bool relative_resource_paths{true};

/// Write `contenteditable` output, which back-translation reads edits from.
/// Write the editing scaffolding: the page's editing state, the address an
/// edit operation names, and the editor script. The mode itself starts off -
/// the host turns it on with `odr.editing.enable()`.
bool editable{false};

/// Whether the view's scripts take the keys that move the selection: the
/// arrows, Tab, Escape, and the keys that open an editor over it.
bool keyboard_navigation{true};
/// Whether the view's scripts take the editing chords: undo and redo.
bool keyboard_shortcuts{true};

/// Render a text document as fixed-size pages rather than reflowing text.
bool text_document_margin{false};

Expand Down
36 changes: 34 additions & 2 deletions src/odr/internal/html/document.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,21 @@ void write_head(const Document &document, const WritingState &state,
out.write_header_end();
}

/// The key classes the view's scripts may take; `editing.md` decision 12.
std::string keyboard_classes(const HtmlConfig &config) {
std::string classes;
if (config.keyboard_navigation) {
classes += "navigation";
}
if (config.keyboard_shortcuts) {
if (!classes.empty()) {
classes += " ";
}
classes += "shortcuts";
}
return classes;
}

void write_body_begin(const Document &document, const WritingState &state) {
HtmlWriter &out = state.out();

Expand All @@ -222,7 +237,18 @@ void write_body_begin(const Document &document, const WritingState &state) {
}
}

out.write_body_begin(HtmlElementOptions().set_class(body_clazz));
out.write_body_begin(
HtmlElementOptions()
.set_class(body_clazz)
.set_attributes([&](const HtmlAttributeWriterCallback &clb) {
// what `enable()` answers, stated only by a render that edits
if (state.config().editable) {
clb("data-odr-editable",
state.document_editable() ? "true" : "readOnly");
}
// not an editing fact: a read-only sheet has a pin to clear
clb("data-odr-keyboard", keyboard_classes(state.config()));
}));

if (paged_content) {
out.write_element_begin("div", HtmlElementOptions().set_class("odr-pages"));
Expand All @@ -237,9 +263,15 @@ void write_body_end(const Document &document, const WritingState &state) {
}

write_search_script(state);
write_document_script(state);
write_editing_script(state);
if (state.config().editable) {
write_document_script(state);
}
if (document.document_type() == DocumentType::spreadsheet) {
write_spreadsheet_script(state);
if (state.config().editable) {
write_sheet_editing_script(state);
}
}
write_viewport_script(state);

Expand Down
23 changes: 11 additions & 12 deletions src/odr/internal/html/document_element.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -233,21 +233,22 @@ std::optional<double> sheet_print_fit(const Sheet &sheet,
return printable / content;
}

/// A run whose style the box around it can carry instead. Not a background, a
/// raised run or an editable one: each means something else on the box.
/// Whether @p element carries `contenteditable`: an editable run of a view
/// that writes its editing into the markup.
bool writes_editable(const Element &element, const html::WritingState &state) {
/// Whether @p element carries the address an edit operation names.
/// `contenteditable` is not written: the mode adds it to these runs.
bool writes_edit_markup(const Element &element,
const html::WritingState &state) {
return state.editable_markup() && state.config().editable &&
element.is_editable();
}

/// A run whose style the box around it can carry instead. Not a background, a
/// raised run or an addressed one: each means something else on the box.
std::optional<Text> plain_text(const Element &element,
const html::WritingState &state) {
if (element.type() != ElementType::text) {
return {};
}
if (writes_editable(element, state)) {
if (writes_edit_markup(element, state)) {
return {};
}

Expand Down Expand Up @@ -487,9 +488,6 @@ void html::translate_sheet(const Sheet &sheet, const WritingState &state) {
HtmlElementOptions()
.set_class("odr-sheet")
.set_attributes([&](const HtmlAttributeWriterCallback &clb) {
// what the editor asks before the user clicks anything
clb("data-odr-editable",
state.document_editable() ? "true" : "readOnly");
// every op names its sheet, and a view holds only one
clb("data-odr-sheet", std::to_string(sheet_ordinal(sheet)));
})
Expand Down Expand Up @@ -669,7 +667,9 @@ void html::translate_sheet(const Sheet &sheet, const WritingState &state) {
const std::optional<FoldedCell> folded = fold_cell(
cell, sheet_state, wraps, anchors_shapes, table_row_style.height);

const char *lock = cell_lock(cell, anchors_shapes);
// scaffolding: a read-only render states no lock
const char *lock =
state.config().editable ? cell_lock(cell, anchors_shapes) : nullptr;

state.out().write_element_begin(
"td",
Expand Down Expand Up @@ -778,8 +778,7 @@ void html::translate_text(const Element &element, const WritingState &state) {
HtmlElementOptions()
.set_inline(true)
.set_attributes([&](const HtmlAttributeWriterCallback &clb) {
if (writes_editable(element, state)) {
clb("contenteditable", "true");
if (writes_edit_markup(element, state)) {
clb("data-odr-path", element.document_path().to_string());
}
})
Expand Down
Loading
Loading