mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-10-31 21:48:09 -06:00
fix(edit, ui): alignments follow the right rows; Format by keyboard keeps the box
Four holes CodeRabbit found on 648dd428:
- alignment_frame framed a one-row paragraph when any alignment entry
was set, including one past the draft's rows, and then moved rows
with no alignment of their own to the column's edge. It now takes the
draft and frames only when every draft row is aligned, by its own
alignment or the paragraph's.
- remap could not tell which of two same paragraphs an edit took
("A\nA" to "A"), and kept the deleted one's alignment. remap_at takes
the caret where the edit ended, and the typing path (follow_typing,
both editors) passes it from the box's state. The caret only chooses
between readings that fit both texts.
- Both editors judged a focus loss by where a click landed, so focusing
a Format text control by keyboard or a screen reader (no pointer)
closed the box before the alignment was chosen. They now also judge
by where the keyboard went (focus_pos): focus outside the document
view, or on the added-text box's own buttons, keeps the box open as a
click there does.
Each change has a test seen failing before it.
This commit is contained in:
@@ -69,13 +69,33 @@ fn trimmed(mut aligns: Vec<Option<Align>>) -> Vec<Option<Align>> {
|
||||
/// paragraph it changed, or for lines only inserted, of the paragraph before them (so Enter in or
|
||||
/// after a centred paragraph makes centred ones).
|
||||
pub fn remap(old: &str, new: &str, aligns: &[Option<Align>]) -> Vec<Option<Align>> {
|
||||
remap_at(old, new, aligns, None)
|
||||
}
|
||||
|
||||
/// [`remap`] for an edit that left the caret at byte `caret` of `new`. Repeated paragraphs can
|
||||
/// make where the edit was ambiguous ("A\nA" to "A" deletes either one); the caret, at the edit's
|
||||
/// end, says which. It only chooses between readings that fit the texts, so a caret away from the
|
||||
/// edit never makes it an edit of other paragraphs.
|
||||
pub fn remap_at(old: &str, new: &str, aligns: &[Option<Align>], caret: Option<usize>) -> Vec<Option<Align>> {
|
||||
if aligns.is_empty() || old == new {
|
||||
return aligns.to_vec();
|
||||
}
|
||||
let (o, n): (Vec<&str>, Vec<&str>) = (old.split('\n').collect(), new.split('\n').collect());
|
||||
let shortest = o.len().min(n.len());
|
||||
let prefix = o.iter().zip(&n).take_while(|(a, b)| a == b).count().min(shortest);
|
||||
let suffix = o.iter().rev().zip(n.iter().rev()).take_while(|(a, b)| a == b).count().min(shortest - prefix);
|
||||
let same_start = o.iter().zip(&n).take_while(|(a, b)| a == b).count().min(shortest);
|
||||
let same_end = o.iter().rev().zip(n.iter().rev()).take_while(|(a, b)| a == b).count().min(shortest);
|
||||
let fewest_kept = shortest - same_start;
|
||||
let (prefix, suffix) = match caret {
|
||||
Some(at) if fewest_kept <= same_end => {
|
||||
// The edit's paragraphs of `new` end before the caret's paragraph when the caret starts
|
||||
// one, and with it otherwise.
|
||||
let starts = at == 0 || at.checked_sub(1).and_then(|i| new.as_bytes().get(i)) == Some(&b'\n');
|
||||
let end = paragraph_at(new, at).saturating_add(usize::from(!starts));
|
||||
let suffix = n.len().saturating_sub(end).clamp(fewest_kept, same_end);
|
||||
(shortest - suffix, suffix)
|
||||
}
|
||||
_ => (same_start, same_end.min(shortest - same_start)),
|
||||
};
|
||||
let at = |i: usize| aligns.get(i).copied().flatten();
|
||||
let inserted_only = o.len() - prefix - suffix == 0;
|
||||
let changed = if inserted_only && prefix > 0 { at(prefix - 1) } else { at(prefix) };
|
||||
@@ -132,4 +152,23 @@ mod tests {
|
||||
assert_eq!(remap("A\nA", "A", &[Some(Center), Some(Right)]), [Some(Center)]);
|
||||
assert_eq!(remap("A", "A\nA", &[Some(Center)]), [Some(Center), Some(Center)]);
|
||||
}
|
||||
|
||||
/// Repeated rows make the edit's place ambiguous: "A\nA" to "A" deletes either row. The caret,
|
||||
/// where the edit ended, settles which (CodeRabbit on 648dd428).
|
||||
#[test]
|
||||
fn the_caret_says_which_of_two_same_rows_an_edit_took() {
|
||||
let aligns = [Some(Center), Some(Right)];
|
||||
assert_eq!(remap_at("A\nA", "A", &aligns, Some(0)), [Some(Right)], "the first row and its break deleted");
|
||||
assert_eq!(remap_at("A\nA", "A", &aligns, Some(1)), [Some(Center)], "the break and the second row deleted");
|
||||
assert_eq!(remap_at("A\nA", "A", &aligns, None), [Some(Center)], "no caret: as before");
|
||||
// Enter at the start of the first of two same rows: the new row is before it.
|
||||
assert_eq!(remap_at("A\nA", "\nA\nA", &aligns, Some(1)), [Some(Center), Some(Center), Some(Right)]);
|
||||
// Enter at the end of the second: the new row follows it.
|
||||
assert_eq!(remap_at("A\nA", "A\nA\n", &aligns, Some(4)), [Some(Center), Some(Right), Some(Right)]);
|
||||
// A caret away from the edit never makes it an edit of other rows.
|
||||
assert_eq!(remap_at("A\nB\nC", "A\nB\nCx", &[Some(Center), Some(Right), None], Some(0)), [Some(Center), Some(Right)]);
|
||||
// A caret past the end, or not on a character boundary, is no panic.
|
||||
assert_eq!(remap_at("A\nA", "A", &aligns, Some(99)), [Some(Center)]);
|
||||
assert_eq!(remap_at("é\né", "é", &aligns, Some(1)), [Some(Center)]);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7255,6 +7255,22 @@ fn a_justified_paragraph_from_the_editor_fills_its_rows() {
|
||||
assert!(row("end")[2] < right - 20.0, "the last row isn't");
|
||||
}
|
||||
|
||||
/// An alignment for a row the draft doesn't have names no row: a one-row title given one for a
|
||||
/// second row only stays where it is, not moved to its column's edge. Nor does a row of the draft
|
||||
/// with no alignment of its own move because another row has one (CodeRabbit on 648dd428).
|
||||
#[test]
|
||||
fn a_title_moves_to_its_column_only_for_an_alignment_of_its_own() {
|
||||
let page = "BT /F1 24 Tf 1 0 0 1 250 700 Tm (Title) Tj ET BT /F1 10 Tf 12 TL 72 650 Td (The body of the page runs across the whole column here.) Tj T* (It has a second row that is a little shorter.) Tj ET";
|
||||
let title = |doc: &Document| text::text_lines(doc, 0).unwrap().into_iter().find(|l| l.text == "Title").unwrap().rect;
|
||||
let before = title(&text_page(page));
|
||||
for draft in ["Title", "Title\nSub"] {
|
||||
let mut doc = text_page(page);
|
||||
let style = text::BlockStyle { aligns: vec![None, Some(added::Align::Right)], ..Default::default() };
|
||||
text::rewrite_block(&mut doc, 0, 0, Some(draft), &style).unwrap();
|
||||
assert!((title(&doc)[0] - before[0]).abs() < 0.5, "{draft:?}: the title stays put, {:?} vs {before:?}", title(&doc));
|
||||
}
|
||||
}
|
||||
|
||||
/// A column whose rows are several runs (a second style on the same baseline) reaches the right
|
||||
/// edge of its whole rows, not of the run at the left edge: a title wider than that first run is
|
||||
/// still held by the column and aligns to it (CodeRabbit on b9940d7d).
|
||||
|
||||
@@ -766,7 +766,7 @@ pub(crate) fn rewrite_block_from(
|
||||
// An alignment for a one-row paragraph is laid out across the column it sits in, as a
|
||||
// paragraph of that width moved to its left edge.
|
||||
let framed;
|
||||
let style = match alignment_frame(&lines, &blocks, &b, style, text.is_some_and(|t| t != b.editor_text)) {
|
||||
let style = match alignment_frame(&lines, &blocks, &b, style, text.unwrap_or(&b.editor_text)) {
|
||||
Some([left, right]) => {
|
||||
framed = BlockStyle { width: Some(right - left), offset: Some([left - b.rect[0], 0.0]), ..style.clone() };
|
||||
&framed
|
||||
|
||||
@@ -50,15 +50,12 @@ fn neighbour_left(lines: &[TextLine], block: &TextBlock) -> Option<f64> {
|
||||
/// to align in: left-aligning a title centred on the page left it where it was, and since a single
|
||||
/// row grows to the page's margin, centring it again moved it right each time. `None` when no
|
||||
/// alignment is asked for, when the paragraph is several rows, turned, or given a width or a move of
|
||||
/// its own, or when its text changes and no column holds it (`text_changed`).
|
||||
pub(crate) fn alignment_frame(
|
||||
lines: &[TextLine],
|
||||
blocks: &[TextBlock],
|
||||
block: &TextBlock,
|
||||
style: &BlockStyle,
|
||||
text_changed: bool,
|
||||
) -> Option<[f64; 2]> {
|
||||
let asked = style.align.is_some() || style.aligns.iter().any(Option::is_some);
|
||||
/// its own, or when its `draft` text changes and no column holds it.
|
||||
pub(crate) fn alignment_frame(lines: &[TextLine], blocks: &[TextBlock], block: &TextBlock, style: &BlockStyle, draft: &str) -> Option<[f64; 2]> {
|
||||
// Every row of the draft is aligned, by its own alignment or the paragraph's: moving the
|
||||
// paragraph to its column's edge would move a row that has none. Entries past the draft's
|
||||
// rows name no row.
|
||||
let asked = (0..crate::paragraph_align::count(draft)).all(|row| style.aligns.get(row).copied().flatten().or(style.align).is_some());
|
||||
if !asked || style.width.is_some() || style.offset.is_some() || block.rows.len() != 1 || block.rotation.abs() > 0.01 {
|
||||
return None;
|
||||
}
|
||||
@@ -88,7 +85,7 @@ pub(crate) fn alignment_frame(
|
||||
// the row keeps the width it would have had without the alignment.
|
||||
match frame {
|
||||
Some(frame) => Some(frame),
|
||||
None if text_changed => None,
|
||||
None if draft != block.editor_text => None,
|
||||
None => Some(own),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -619,8 +619,9 @@ pub(crate) fn editor(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, ad
|
||||
// Clicks outside the document (the Format text panel) keep the text open, as they keep a
|
||||
// paragraph being edited open.
|
||||
// Where the click that took the focus was: the pointer itself may be gone by then (a touch, a
|
||||
// pen lifted away).
|
||||
let outside = ctx.input(|i| i.pointer.interact_pos()).is_some_and(|p| !view.viewport_rect().contains(p));
|
||||
// pen lifted away). The keyboard moving there (Tab, a screen reader) keeps it open too.
|
||||
let away = |p: egui::Pos2| !view.viewport_rect().contains(p);
|
||||
let outside = ctx.input(|i| i.pointer.interact_pos()).is_some_and(away) || crate::edit_text_ui::focus_pos(ctx).is_some_and(away);
|
||||
// Shown at once, like the paragraph editor: a fade-in shows the page's copy through it. Over
|
||||
// the page, so only where the document shows: text scrolled under the mode bar or a side
|
||||
// panel must leave them their pointer (#2755).
|
||||
@@ -700,8 +701,10 @@ pub(crate) fn editor(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, ad
|
||||
})
|
||||
.response
|
||||
.rect;
|
||||
// Pressing a button takes the focus from the text: that is not a click away.
|
||||
let on_buttons = ui.input(|i| i.pointer.interact_pos()).is_some_and(|p| buttons.contains(p));
|
||||
// Pressing a button, or moving the keyboard to one, takes the focus from the text:
|
||||
// that is not a click away.
|
||||
let on_buttons = ui.input(|i| i.pointer.interact_pos()).is_some_and(|p| buttons.contains(p))
|
||||
|| crate::edit_text_ui::focus_pos(ui.ctx()).is_some_and(|p| buttons.contains(p));
|
||||
cancel |= ui.input(|i| i.key_pressed(egui::Key::Escape));
|
||||
commit = done || resp.lost_focus() && !on_buttons && !outside && !view.objects.hold_editor;
|
||||
});
|
||||
|
||||
@@ -155,6 +155,10 @@ pub(crate) fn follow_typing(
|
||||
aligns: &mut [&mut Vec<Option<pdfcraft_engine::TextAlign>>],
|
||||
) {
|
||||
let now: Vec<Vec<_>> = aligns.iter().map(|a| a.to_vec()).collect();
|
||||
// Where the edit left the caret, as a byte of `after`: it says which of two same rows changed.
|
||||
let caret = egui::TextEdit::load_state(ctx, id)
|
||||
.and_then(|s| s.cursor.char_range())
|
||||
.map(|r| after.char_indices().nth(r.primary.index.into()).map_or(after.len(), |(at, _)| at));
|
||||
let next = ctx.data_mut(|d| {
|
||||
let seen = d.get_temp_mut_or_default::<AlignHistory>(id.with(ALIGN_HISTORY));
|
||||
if seen.iter().any(|(text, was)| text == before && *was != now) {
|
||||
@@ -163,7 +167,7 @@ pub(crate) fn follow_typing(
|
||||
remember(seen, before, now.clone());
|
||||
let next = match seen.iter().rev().find(|(text, _)| text == after) {
|
||||
Some((_, was)) if was.len() == now.len() => was.clone(),
|
||||
_ => now.iter().map(|a| pdfcraft_engine::paragraph_align::remap(before, after, a)).collect(),
|
||||
_ => now.iter().map(|a| pdfcraft_engine::paragraph_align::remap_at(before, after, a, caret)).collect(),
|
||||
};
|
||||
remember(seen, after, next.clone());
|
||||
next
|
||||
@@ -624,9 +628,11 @@ pub(crate) fn page_input(cx: PageToolContext<'_>, lines: &[pdfcraft_engine::Text
|
||||
/// The inline editor; returns the edit once the text is applied.
|
||||
pub(crate) fn overlay(ctx: &egui::Context, view: &mut DocView, info: &DocInfo) -> Option<Edit> {
|
||||
let view_id = view.id;
|
||||
// Clicks outside the document (the Format text panel) keep the paragraph open. Where the
|
||||
// click landed is `interact_pos`: `latest_pos` is gone on the frame a tap lifts.
|
||||
let outside = ctx.input(|i| i.pointer.interact_pos()).is_some_and(|p| !view.viewport_rect().contains(p));
|
||||
// Clicks outside the document (the Format text panel) keep the paragraph open, as does the
|
||||
// keyboard moving there. Where the click landed is `interact_pos`: `latest_pos` is gone on the
|
||||
// frame a tap lifts.
|
||||
let away = |p: egui::Pos2| !view.viewport_rect().contains(p);
|
||||
let outside = ctx.input(|i| i.pointer.interact_pos()).is_some_and(away) || focus_pos(ctx).is_some_and(away);
|
||||
let page = view.line_editor.as_ref()?.page;
|
||||
let xf = view.page_xform(page)?;
|
||||
let hold = view.objects.hold_editor;
|
||||
@@ -821,6 +827,13 @@ pub(crate) fn finished_edit(ed: LineEditor) -> Option<Edit> {
|
||||
})
|
||||
}
|
||||
|
||||
/// Where the keyboard is: the centre of the widget that has it. A text box that loses it to a
|
||||
/// widget reached by Tab or a screen reader (the Format text panel's controls) judges where it
|
||||
/// went by this, as it judges a click by where the click landed.
|
||||
pub(crate) fn focus_pos(ctx: &egui::Context) -> Option<egui::Pos2> {
|
||||
ctx.memory(|m| m.focused()).and_then(|id| ctx.read_response(id)).map(|r| r.rect.center())
|
||||
}
|
||||
|
||||
/// Whether an editor's text box `r` lost the keyboard this frame, given whether it `had` it at
|
||||
/// the end of the editor's last frame (updated here). egui's `lost_focus` holds for a second
|
||||
/// frame as well, and an editor that judges a click away by where the click landed must judge it
|
||||
|
||||
@@ -1014,6 +1014,77 @@ fn aligning_a_selected_line_of_added_text_aligns_only_that_line() {
|
||||
assert_eq!(aligns(&h), (pdfcraft_engine::TextAlign::Left, vec![]));
|
||||
}
|
||||
|
||||
/// Two lines of added text with the same words, each aligned its own way: deleting the first line
|
||||
/// with its break leaves the second line's alignment, not the deleted one's (CodeRabbit on
|
||||
/// 648dd428).
|
||||
#[test]
|
||||
fn deleting_the_first_of_two_same_lines_keeps_the_second_lines_alignment() {
|
||||
use pdfcraft_engine::TextAlign::{Center, Right};
|
||||
let mut h = harness();
|
||||
let rect = [40.0, 195.0, 240.0, 220.0];
|
||||
let text = pdfcraft_engine::AddedText { rect, text: "Same\nSame".into(), aligns: vec![Some(Center), Some(Right)], ..Default::default() };
|
||||
assert!(h.state_mut().apply_edit(pdfcraft_engine::Edit::AddText { page: 0, text }));
|
||||
assert!(h.state_mut().execute("edit.text"));
|
||||
h.run_steps(3);
|
||||
let point = at(&h, 120.0, 195.0);
|
||||
click(&mut h, point);
|
||||
assert_eq!(h.state().views[0].content.draft.as_ref().and_then(|d| d.index), Some(0), "the editor opened on the text");
|
||||
// Select the first line with its break, and delete it.
|
||||
h.key_press(egui::Key::ArrowUp);
|
||||
h.key_press(egui::Key::Home);
|
||||
// "Same" and its line break.
|
||||
for _ in 0..5 {
|
||||
h.key_press_modifiers(egui::Modifiers::SHIFT, egui::Key::ArrowRight);
|
||||
}
|
||||
h.key_press(egui::Key::Backspace);
|
||||
h.run_steps(2);
|
||||
let d = h.state().views[0].content.draft.as_ref().expect("still typing");
|
||||
assert_eq!((d.text.as_str(), d.style.aligns.clone()), ("Same", vec![Some(Right)]), "the second line's alignment stays");
|
||||
}
|
||||
|
||||
/// The same with the alignment controls reached by keyboard or a screen reader (focused, then
|
||||
/// pressed, with no pointer): the box stays open when the focus leaves it for the Format text
|
||||
/// panel, and both choices are for the selected line.
|
||||
#[test]
|
||||
fn aligning_a_selected_line_of_added_text_by_keyboard_keeps_the_box_open() {
|
||||
let mut h = harness();
|
||||
let rect = [40.0, 195.0, 240.0, 220.0];
|
||||
let text = pdfcraft_engine::AddedText {
|
||||
rect,
|
||||
text: "Line one
|
||||
Line two"
|
||||
.into(),
|
||||
..Default::default()
|
||||
};
|
||||
assert!(h.state_mut().apply_edit(pdfcraft_engine::Edit::AddText { page: 0, text }));
|
||||
assert!(h.state_mut().execute("edit.text"));
|
||||
h.run_steps(3);
|
||||
let point = at(&h, 45.0, 195.0);
|
||||
click(&mut h, point);
|
||||
assert_eq!(h.state().views[0].content.draft.as_ref().and_then(|d| d.index), Some(0), "the editor opened on the text");
|
||||
h.key_press_modifiers(egui::Modifiers::COMMAND, egui::Key::End);
|
||||
h.key_press_modifiers(egui::Modifiers::SHIFT, egui::Key::Home);
|
||||
h.run_steps(2);
|
||||
let align_by_focus = |h: &mut Harness<'static, PdfCraftApp>, choice: &str| {
|
||||
for label in ["Alignment", choice] {
|
||||
h.get_by_label(label).focus();
|
||||
h.run_steps(2);
|
||||
h.get_by_label(label).click();
|
||||
h.run_steps(3);
|
||||
}
|
||||
};
|
||||
let aligns = |h: &Harness<'static, PdfCraftApp>| match &added(h)[0].content {
|
||||
pdfcraft_engine::AddedContent::Text(t) => (t.align, t.aligns.clone()),
|
||||
_ => panic!(),
|
||||
};
|
||||
align_by_focus(&mut h, "Align right");
|
||||
assert!(h.state().views[0].content.draft.is_some(), "the box stays open");
|
||||
assert_eq!(aligns(&h), (pdfcraft_engine::TextAlign::Left, vec![None, Some(pdfcraft_engine::TextAlign::Right)]));
|
||||
align_by_focus(&mut h, "Centre");
|
||||
assert!(h.state().views[0].content.draft.is_some(), "the box is still open");
|
||||
assert_eq!(aligns(&h), (pdfcraft_engine::TextAlign::Left, vec![None, Some(pdfcraft_engine::TextAlign::Center)]));
|
||||
}
|
||||
|
||||
/// The same in an added text box: Redo after Undo puts the typing back in the box, and the
|
||||
/// document's own Redo stays waiting.
|
||||
#[test]
|
||||
|
||||
@@ -6578,6 +6578,39 @@ fn aligning_a_selected_row_aligns_only_that_row_and_undo_puts_it_back() {
|
||||
assert!((row_rect(&h, "nine")[0] - last[0]).abs() < 0.5, "the last row is back");
|
||||
}
|
||||
|
||||
/// Format text's alignment flyout by keyboard or screen reader: each control is focused, then
|
||||
/// pressed, with no pointer anywhere.
|
||||
fn align_by_focus(h: &mut Harness<'static, PdfCraftApp>, choice: &str) {
|
||||
for label in ["Alignment", choice] {
|
||||
h.get_by_label(label).focus();
|
||||
h.run_steps(2);
|
||||
h.get_by_label(label).click();
|
||||
h.run_steps(3);
|
||||
}
|
||||
}
|
||||
|
||||
/// The same as by pointer, with the alignment controls reached by keyboard or a screen reader:
|
||||
/// the paragraph stays open when the focus leaves it for the Format text panel, the selected row
|
||||
/// takes the alignment, and a second choice is for that row again.
|
||||
#[test]
|
||||
fn aligning_a_selected_row_by_keyboard_keeps_the_paragraph_open() {
|
||||
let mut h = three_row_paragraph();
|
||||
let first = row_rect(&h, "One");
|
||||
let wide = row_rect(&h, "three");
|
||||
h.key_press_modifiers(Modifiers::COMMAND, Key::End);
|
||||
h.key_press_modifiers(Modifiers::SHIFT, Key::Home);
|
||||
h.run_steps(2);
|
||||
align_by_focus(&mut h, "Align right");
|
||||
assert!(h.state().views[0].line_editor.is_some(), "the paragraph stays open");
|
||||
let moved = row_rect(&h, "nine");
|
||||
assert!((moved[2] - wide[2]).abs() < 1.0, "the last row is right-aligned: {moved:?}");
|
||||
align_by_focus(&mut h, "Centre");
|
||||
assert!(h.state().views[0].line_editor.is_some(), "the paragraph is still open");
|
||||
assert!((row_rect(&h, "One")[0] - first[0]).abs() < 0.5, "the first row stays put: {:?}", row_rect(&h, "One"));
|
||||
let last = row_rect(&h, "nine");
|
||||
assert!((((last[0] + last[2]) - (wide[0] + wide[2])) / 2.0).abs() < 1.0, "the last row is centred: {last:?} in {wide:?}");
|
||||
}
|
||||
|
||||
/// A row aligned on its own, deleted by typing, then brought back with Undo from the menu: it
|
||||
/// comes back with its alignment, and Redo deletes it again. The box's history keeps only text,
|
||||
/// so the restored row took the alignment of the row before it.
|
||||
|
||||
Reference in New Issue
Block a user