Skip to content

Commit e376f27

Browse files
timabellclaude
andcommitted
fix(dioxus): Prevent list item editing from losing trailing newlines
When editing a list item bullet in Dioxus, making no changes, and exiting focus, the bullet would be combined with the following bullet (losing the newline between them). Root cause: In block.rs for ListItem editing, the content was extracted from `line.full` and `.trim_end()` was applied, removing the trailing newline. But when committing, EditorBlock used `block.node_range` which included the newline. This mismatch caused the newline to be lost. Changes: - Add optional `edit_range` parameter to EditorBlock that defaults to `block.node_range` but allows callers to override it - Remove `.trim_end()` from ListItem content extraction in block.rs - Pass `line.full` as the `edit_range` for ListItem so range matches content - Add no-op check to skip commits when content hasn't changed - Rename `block_range` to `edit_range` in helper functions for clarity - Add test verifying `line.full` includes trailing newline (round-trip invariant) Prompts: - "dioxus editing is a bit hosed. editing a bullet, changing nothing and exiting focus results in the bullet being combined with the following bullet (i.e. loses the newline between them)" Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent d65fe1e commit e376f27

3 files changed

Lines changed: 82 additions & 23 deletions

File tree

crates/markdown-neuraxis-dioxus/src/ui/components/block.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,12 +78,11 @@ pub fn BlockRenderer(
7878
BlockKind::ListItem { .. } => {
7979
if is_focused {
8080
// Use first line only - node_range includes nested children
81-
let content_text = block
82-
.lines
83-
.first()
81+
let first_line = block.lines.first();
82+
let edit_range = first_line.map(|line| line.full.clone());
83+
let content_text = first_line
8484
.and_then(|line| source.get(line.full.clone()))
8585
.unwrap_or("")
86-
.trim_end()
8786
.to_string();
8887
let block_clone = block.clone();
8988
rsx! {
@@ -92,6 +91,7 @@ pub fn BlockRenderer(
9291
EditorBlock {
9392
block: block_clone,
9493
content_text,
94+
edit_range,
9595
on_command,
9696
on_cancel: {
9797
let mut focused_anchor_id = focused_anchor_id;

crates/markdown-neuraxis-dioxus/src/ui/components/editor_block.rs

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,23 +8,31 @@ use markdown_neuraxis_engine::editing::{Block, BlockKind, Cmd};
88
pub fn EditorBlock(
99
block: Block,
1010
content_text: String,
11+
/// Optional override for the edit range. Defaults to block.node_range.
12+
/// Used for ListItems where we edit only the first line, not nested children.
13+
edit_range: Option<std::ops::Range<usize>>,
1114
on_command: Callback<Cmd>,
1215
on_cancel: Callback<()>,
1316
) -> Element {
1417
// Local state for textarea content - only commit changes on specific events
18+
let original_content = content_text.clone();
1519
let local_content = use_signal(|| content_text.clone());
20+
let edit_range = edit_range.unwrap_or_else(|| block.node_range.clone());
1621

1722
// Helper to commit current changes to the document
1823
let commit_changes = {
1924
let on_command = on_command;
20-
let block_range = block.node_range.clone();
25+
let edit_range = edit_range.clone();
2126
move || {
2227
let current_text = local_content.read().clone();
23-
let replace_cmd = Cmd::ReplaceRange {
24-
range: block_range.clone(),
25-
text: current_text,
26-
};
27-
on_command.call(replace_cmd);
28+
// Only commit if content actually changed to avoid no-op replacements
29+
if current_text != original_content {
30+
let replace_cmd = Cmd::ReplaceRange {
31+
range: edit_range.clone(),
32+
text: current_text,
33+
};
34+
on_command.call(replace_cmd);
35+
}
2836
}
2937
};
3038

@@ -50,15 +58,15 @@ pub fn EditorBlock(
5058

5159
// Handle special keyboard commands via keydown (Tab, Shift+Tab, Enter, Escape)
5260
onkeydown: {
53-
let block_range = block.node_range.clone();
61+
let edit_range = edit_range.clone();
5462
let block_kind = block.kind.clone();
5563
let on_command = on_command;
5664
let on_cancel = on_cancel;
5765
let commit_changes = commit_changes.clone();
5866
move |event: Event<KeyboardData>| {
5967
handle_editor_keydown(
6068
event,
61-
&block_range,
69+
&edit_range,
6270
&block_kind,
6371
&on_command,
6472
&on_cancel,
@@ -93,15 +101,15 @@ fn calculate_textarea_rows(content: &str) -> u32 {
93101
// Helper function to handle editor keyboard events
94102
fn handle_editor_keydown(
95103
event: Event<KeyboardData>,
96-
block_range: &std::ops::Range<usize>,
104+
edit_range: &std::ops::Range<usize>,
97105
block_kind: &BlockKind,
98106
on_command: &Callback<Cmd>,
99107
on_cancel: &Callback<()>,
100108
commit_changes: &impl Fn(),
101109
) {
102110
match event.key() {
103-
Key::Tab => handle_tab_key(event, block_range, on_command, commit_changes),
104-
Key::Enter => handle_enter_key(event, block_range, block_kind, on_command, commit_changes),
111+
Key::Tab => handle_tab_key(event, edit_range, on_command, commit_changes),
112+
Key::Enter => handle_enter_key(event, edit_range, block_kind, on_command, commit_changes),
105113
Key::Escape => {
106114
commit_changes();
107115
on_cancel.call(());
@@ -113,7 +121,7 @@ fn handle_editor_keydown(
113121
// Handle Tab key press for indentation
114122
fn handle_tab_key(
115123
event: Event<KeyboardData>,
116-
block_range: &std::ops::Range<usize>,
124+
edit_range: &std::ops::Range<usize>,
117125
on_command: &Callback<Cmd>,
118126
commit_changes: &impl Fn(),
119127
) {
@@ -122,11 +130,11 @@ fn handle_tab_key(
122130

123131
let cmd = if event.modifiers().shift() {
124132
Cmd::OutdentLines {
125-
range: block_range.clone(),
133+
range: edit_range.clone(),
126134
}
127135
} else {
128136
Cmd::IndentLines {
129-
range: block_range.clone(),
137+
range: edit_range.clone(),
130138
}
131139
};
132140
on_command.call(cmd);
@@ -135,7 +143,7 @@ fn handle_tab_key(
135143
// Handle Enter key press for new lines or list item splitting
136144
fn handle_enter_key(
137145
event: Event<KeyboardData>,
138-
block_range: &std::ops::Range<usize>,
146+
edit_range: &std::ops::Range<usize>,
139147
block_kind: &BlockKind,
140148
on_command: &Callback<Cmd>,
141149
commit_changes: &impl Fn(),
@@ -149,11 +157,9 @@ fn handle_enter_key(
149157
commit_changes();
150158

151159
let cmd = match block_kind {
152-
BlockKind::ListItem { .. } => Cmd::SplitListItem {
153-
at: block_range.end,
154-
},
160+
BlockKind::ListItem { .. } => Cmd::SplitListItem { at: edit_range.end },
155161
_ => Cmd::InsertText {
156-
at: block_range.end,
162+
at: edit_range.end,
157163
text: "\n".to_string(),
158164
},
159165
};

crates/markdown-neuraxis-engine/src/editing/snapshot.rs

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2271,4 +2271,57 @@ mod tests {
22712271
panic!("Expected Strong, got {:?}", block.segments[0].kind);
22722272
}
22732273
}
2274+
2275+
#[test]
2276+
fn test_list_item_line_full_includes_trailing_newline() {
2277+
// Verifies that line.full range includes trailing newline, so extracting
2278+
// source[line.full] and replacing range line.full is a no-op.
2279+
// This is critical for the editing workflow where focusing/unfocusing
2280+
// a list item without changes should not modify the document.
2281+
let text = "- Item 1\n- Item 2\n";
2282+
let mut doc = Document::from_bytes(text.as_bytes()).unwrap();
2283+
doc.create_anchors_from_tree();
2284+
let snapshot = doc.snapshot();
2285+
2286+
// Find the first list item
2287+
fn find_first_list_item(blocks: &[Block]) -> Option<&Block> {
2288+
for block in blocks {
2289+
if matches!(block.kind, BlockKind::ListItem { .. }) {
2290+
return Some(block);
2291+
}
2292+
if let BlockContent::Children(children) = &block.content
2293+
&& let Some(found) = find_first_list_item(children)
2294+
{
2295+
return Some(found);
2296+
}
2297+
}
2298+
None
2299+
}
2300+
2301+
let item = find_first_list_item(&snapshot.blocks).expect("Should have list item");
2302+
2303+
// The first line's full range should capture "- Item 1\n" exactly
2304+
assert!(
2305+
!item.lines.is_empty(),
2306+
"List item should have at least one line"
2307+
);
2308+
let first_line = &item.lines[0];
2309+
2310+
// Extract content using line.full (what the UI does for editing)
2311+
let extracted = &text[first_line.full.clone()];
2312+
2313+
// Verify it includes the trailing newline
2314+
assert_eq!(
2315+
extracted, "- Item 1\n",
2316+
"line.full should include trailing newline"
2317+
);
2318+
2319+
// Verify that replacing first_line.full with extracted content is a no-op
2320+
let mut modified = text.to_string();
2321+
modified.replace_range(first_line.full.clone(), extracted);
2322+
assert_eq!(
2323+
modified, text,
2324+
"Replacing line.full with its content should be a no-op"
2325+
);
2326+
}
22742327
}

0 commit comments

Comments
 (0)