From 17c136ef4031d240bad7640328e68df2f54bd8cc Mon Sep 17 00:00:00 2001 From: hz Date: Wed, 15 Jul 2026 20:19:47 -0500 Subject: [PATCH] Fix question review interactions --- scripts/ui/question_player.gd | 124 ++++++++++++++++++------------- tests/post_answer_review_test.gd | 67 +++++++++++++++++ 2 files changed, 141 insertions(+), 50 deletions(-) diff --git a/scripts/ui/question_player.gd b/scripts/ui/question_player.gd index 9e67e4e..5d9633b 100644 --- a/scripts/ui/question_player.gd +++ b/scripts/ui/question_player.gd @@ -24,7 +24,7 @@ var main: Control var theme_service var choice_buttons: Array[Button] = [] var choice_percent_labels: Array[Label] = [] -var choice_top_labels: Array[Label] = [] +var choice_top_labels: Array[RichTextLabel] = [] var choice_followup_panels: Array[PanelContainer] = [] var choice_followup_contents: Array[VBoxContainer] = [] var eliminated: Array = [] @@ -37,9 +37,11 @@ var flagged := false var selected_choice := "" var current_answer := {} var question_highlights := {} +var explanation_highlights := {} var reported_question_ids := [] var stem_plain_text := "" -var highlight_drag_started_in_stem := false +var explanation_plain_text := "" +var highlight_drag_source := "" var timer_color_state := "" var pending_correct_advance := false var question_text_scale := 1.0 @@ -317,18 +319,19 @@ func _ready() -> void: button.add_child(percent_label) choice_percent_labels.append(percent_label) - var top_label := Label.new() + var top_label := RichTextLabel.new() top_label.visible = false top_label.mouse_filter = Control.MOUSE_FILTER_IGNORE - top_label.horizontal_alignment = HORIZONTAL_ALIGNMENT_LEFT - top_label.vertical_alignment = VERTICAL_ALIGNMENT_TOP - top_label.autowrap_mode = TextServer.AUTOWRAP_WORD_SMART + top_label.bbcode_enabled = true + top_label.fit_content = false + top_label.scroll_active = false top_label.set_anchors_and_offsets_preset(Control.PRESET_FULL_RECT) top_label.offset_left = 14 top_label.offset_top = 10 top_label.offset_right = -96 top_label.offset_bottom = -252 - _set_scaled_label(top_label, 16, Style.RED) + _set_scaled_rich_text(top_label, 16) + top_label.add_theme_color_override("default_color", Style.RED) button.add_child(top_label) choice_top_labels.append(top_label) @@ -354,6 +357,7 @@ func _ready() -> void: explanation_label = RichTextLabel.new() explanation_label.bbcode_enabled = true explanation_label.fit_content = true + explanation_label.selection_enabled = true explanation_label.scroll_active = false explanation_label.custom_minimum_size = Vector2.ZERO explanation_label.visible = false @@ -526,10 +530,16 @@ func _input(event: InputEvent) -> void: return if mouse_event.pressed: - highlight_drag_started_in_stem = stem_label.get_global_rect().has_point(mouse_event.global_position) - elif highlight_drag_started_in_stem: - highlight_drag_started_in_stem = false - call_deferred("_commit_current_text_selection") + if stem_label.get_global_rect().has_point(mouse_event.global_position): + highlight_drag_source = "stem" + elif explanation_label.visible and explanation_label.get_global_rect().has_point(mouse_event.global_position): + highlight_drag_source = "explanation" + else: + highlight_drag_source = "" + elif highlight_drag_source != "": + var selection_source := highlight_drag_source + highlight_drag_source = "" + call_deferred("_commit_current_text_selection", selection_source) elif _should_clear_selection_from_click(mouse_event): _clear_selected_choice() @@ -567,6 +577,7 @@ func _load_question() -> void: timer_bar.visible = BlockSessionService.session_kind == "core" _update_timer_bar() stem_plain_text = str(question.get("stem", "")) + explanation_plain_text = str(question.get("explanation", "")) _render_stem() _load_question_media(question) lead_label.text = question.get("lead_in", "") @@ -835,6 +846,7 @@ func _on_random_question_failed(message: String) -> void: func _show_answer_review() -> void: _show_explanation() _show_references(BlockSessionService.get_current_question().get("references", [])) + timer_bar.visible = false next_button.text = "Next" next_button.disabled = false if skip_button != null: @@ -859,8 +871,8 @@ func _show_explanation() -> void: elif id == selected_choice: choice_buttons[index].modulate = Color(1.0, 0.48, 0.48) - var result_text := "Correct" if current_answer.get("is_correct", false) else "Incorrect" - explanation_label.text = "[b]%s[/b]\n%s" % [result_text, question.get("explanation", "")] + explanation_plain_text = str(question.get("explanation", "")) + _render_explanation() explanation_label.visible = true _update_question_elo_label(true) _refresh_nav_palette() @@ -1039,47 +1051,54 @@ func _reset_wrong_feedback() -> void: feedback_fog.color = Color(1.0, 0.0, 0.0, 0.0) -func _commit_current_text_selection() -> void: - if stem_label == null: +func _commit_current_text_selection(source: String = "stem") -> void: + var target_label := stem_label if source == "stem" else explanation_label + if target_label == null: return - var raw_selected_text := stem_label.get_selected_text() + var raw_selected_text := target_label.get_selected_text() var selected_text := raw_selected_text.strip_edges() if selected_text == "": return var left_trimmed_text := raw_selected_text.strip_edges(true, false) var leading_trim_count := raw_selected_text.length() - left_trimmed_text.length() - var selection_start := stem_label.get_selection_from() + leading_trim_count - _highlight_range(selection_start, selection_start + selected_text.length()) + var selection_start := target_label.get_selection_from() + leading_trim_count + _highlight_range(source, selection_start, selection_start + selected_text.length()) -func _highlight_range(start: int, end: int) -> void: +func _highlight_range(source: String, start: int, end: int) -> void: var question_id := str(BlockSessionService.get_current_question().get("id", "")) - if not question_highlights.has(question_id): - question_highlights[question_id] = [] + var highlight_store := question_highlights if source == "stem" else explanation_highlights + var plain_text := stem_plain_text if source == "stem" else explanation_plain_text + var target_label := stem_label if source == "stem" else explanation_label + if not highlight_store.has(question_id): + highlight_store[question_id] = [] - var highlights: Array = question_highlights[question_id] - start = clamp(start, 0, stem_plain_text.length()) - end = clamp(end, start, stem_plain_text.length()) + var highlights: Array = highlight_store[question_id] + start = clamp(start, 0, plain_text.length()) + end = clamp(end, start, plain_text.length()) if start == end: return if _subtract_highlight_range(highlights, start, end): - _render_stem() - if stem_label.has_method("deselect"): - stem_label.deselect() + _render_highlighted_text(source) + if target_label.has_method("deselect"): + target_label.deselect() return highlights.append({"start": start, "end": end}) - _render_stem() - if stem_label.has_method("deselect"): - stem_label.deselect() + _render_highlighted_text(source) + if target_label.has_method("deselect"): + target_label.deselect() func _clear_current_highlights() -> void: var question_id := str(BlockSessionService.get_current_question().get("id", "")) question_highlights[question_id] = [] + explanation_highlights[question_id] = [] _render_stem() + if explanation_label.visible: + _render_explanation() func _toggle_report_current_question() -> void: @@ -1923,22 +1942,33 @@ func _format_lab_value(lab: Dictionary) -> String: func _render_stem() -> void: + _render_highlighted_text("stem") + + +func _render_explanation() -> void: + _render_highlighted_text("explanation") + + +func _render_highlighted_text(source: String) -> void: var question_id := str(BlockSessionService.get_current_question().get("id", "")) - var ranges: Array = question_highlights.get(question_id, []).duplicate() + var highlight_store := question_highlights if source == "stem" else explanation_highlights + var plain_text := stem_plain_text if source == "stem" else explanation_plain_text + var target_label := stem_label if source == "stem" else explanation_label + var ranges: Array = highlight_store.get(question_id, []).duplicate() ranges.sort_custom(func(a: Dictionary, b: Dictionary): return int(a["start"]) < int(b["start"])) var rendered := "" var cursor := 0 for highlight_range in ranges: - var start: int = clamp(int(highlight_range.get("start", 0)), 0, stem_plain_text.length()) - var end: int = clamp(int(highlight_range.get("end", 0)), start, stem_plain_text.length()) + var start: int = clamp(int(highlight_range.get("start", 0)), 0, plain_text.length()) + var end: int = clamp(int(highlight_range.get("end", 0)), start, plain_text.length()) if start < cursor: continue - rendered += _escape_bbcode(stem_plain_text.substr(cursor, start - cursor)) - rendered += "[bgcolor=yellow][color=black]%s[/color][/bgcolor]" % _escape_bbcode(stem_plain_text.substr(start, end - start)) + rendered += _escape_bbcode(plain_text.substr(cursor, start - cursor)) + rendered += "[bgcolor=yellow][color=black]%s[/color][/bgcolor]" % _escape_bbcode(plain_text.substr(start, end - start)) cursor = end - rendered += _escape_bbcode(stem_plain_text.substr(cursor)) - stem_label.text = rendered + rendered += _escape_bbcode(plain_text.substr(cursor)) + target_label.text = rendered func _escape_bbcode(value: String) -> String: @@ -2015,6 +2045,7 @@ func _set_choice_button_text(index: int, choice: Dictionary) -> void: var choice_text := str(choice.get("text", "")) var display_label := _display_label_for_index(index) var has_visible_followup := index < choice_followup_panels.size() and choice_followup_panels[index].visible + choice_top_labels[index].offset_bottom = -252 if has_visible_followup else -8 choice_buttons[index].custom_minimum_size = _choice_row_size(index, choice_id, has_visible_followup) _set_choice_percent_label(index, "") _set_choice_top_label(index, "") @@ -2025,17 +2056,19 @@ func _set_choice_button_text(index: int, choice: Dictionary) -> void: if has_visible_followup: var wrong_text := "%s. %s" % [display_label, choice_text] choice_buttons[index].text = "" - _set_choice_top_label(index, wrong_text) + _set_choice_top_label(index, _escape_bbcode(wrong_text)) else: - var wrong_text := "%s. %s" % [display_label, _strike_text(choice_text)] - choice_buttons[index].text = wrong_text + var wrong_text := "%s. %s" % [display_label, choice_text] + choice_buttons[index].text = "" + _set_choice_top_label(index, "[s]%s[/s]" % _escape_bbcode(wrong_text)) elif _should_show_correct_choice_percentage(choice_id): var correct_percentage := _get_choice_selection_percentage(choice_id) if correct_percentage >= 0: _set_choice_percent_label(index, "%d%%" % correct_percentage) choice_buttons[index].text = "%s. %s" % [display_label, choice_text] elif eliminated.has(choice_id): - choice_buttons[index].text = "%s. [eliminated] %s" % [display_label, _strike_text(choice_text)] + choice_buttons[index].text = "" + _set_choice_top_label(index, "[s]%s. %s[/s]" % [display_label, _escape_bbcode(choice_text)]) else: choice_buttons[index].text = "%s. %s" % [display_label, choice_text] @@ -2297,7 +2330,6 @@ func _build_collapsed_followup(index: int, state: Dictionary) -> void: Style.style_chip(summary, Style.AMBER) UISoundService.set_button_sound(summary, "open") _set_scaled_button_font(summary, 15) - summary.gui_input.connect(func(_event: InputEvent): _mark_review_input_handled()) summary.pressed.connect(func(choice_id := str(state.get("choice_id", ""))): _toggle_followup_state(choice_id)) content.add_child(summary) @@ -2311,7 +2343,6 @@ func _build_readonly_followup(index: int, state: Dictionary) -> void: Style.style_chip(summary, Style.AMBER) UISoundService.set_button_sound(summary, "close") _set_scaled_button_font(summary, 15) - summary.gui_input.connect(func(_event: InputEvent): _mark_review_input_handled()) summary.pressed.connect(func(choice_id := str(state.get("choice_id", ""))): _toggle_followup_state(choice_id)) content.add_child(summary) @@ -2446,13 +2477,6 @@ func _update_question_elo_label(show_label: bool) -> void: question_elo_label.visible = true -func _strike_text(value: String) -> String: - var result := "" - for character in value: - result += character + char(0x0336) - return result - - func _make_labs_icon() -> Texture2D: var image := Image.create(24, 24, false, Image.FORMAT_RGBA8) image.fill(Color(0, 0, 0, 0)) diff --git a/tests/post_answer_review_test.gd b/tests/post_answer_review_test.gd index 9b05e63..e6e54d2 100644 --- a/tests/post_answer_review_test.gd +++ b/tests/post_answer_review_test.gd @@ -8,6 +8,9 @@ func _initialize() -> void: _test_reference_url_filter() _test_correct_feedback_does_not_auto_advance() _test_correct_review_locks_answer_interactions() + _test_eliminated_choice_uses_rich_text_strikeout() + _test_explanation_highlighting_has_no_status_heading() + _test_collapsed_followup_review_expands() quit(1 if failures > 0 else 0) @@ -74,6 +77,70 @@ func _test_correct_review_locks_answer_interactions() -> void: player.free() +func _test_eliminated_choice_uses_rich_text_strikeout() -> void: + var player = load("res://scripts/ui/question_player.gd").new() + player.choice_buttons.append(Button.new()) + player.choice_percent_labels.append(Label.new()) + var strike_label := RichTextLabel.new() + strike_label.bbcode_enabled = true + player.choice_top_labels.append(strike_label) + player.choice_followup_panels.append(PanelContainer.new()) + player.eliminated = ["A"] + player.presented_choices = [{"id": "A", "text": "Observation only"}] + player._set_choice_button_text(0, player.presented_choices[0]) + _assert(player.choice_buttons[0].text == "", "eliminated choice should not use combining characters in button text") + _assert(strike_label.text == "[s]A. Observation only[/s]", "eliminated choice should use native rich-text strikeout") + _assert(not "[eliminated]" in strike_label.text, "eliminated choice should not include a redundant label") + player.choice_buttons[0].free() + player.choice_percent_labels[0].free() + strike_label.free() + player.choice_followup_panels[0].free() + player.free() + + +func _test_explanation_highlighting_has_no_status_heading() -> void: + var player = load("res://scripts/ui/question_player.gd").new() + player.explanation_label = RichTextLabel.new() + player.explanation_label.bbcode_enabled = true + player.explanation_plain_text = "The key finding supports observation." + player.explanation_highlights[""] = [{"start": 4, "end": 15}] + player._render_explanation() + _assert(not player.explanation_label.text.begins_with("[b]Correct"), "explanation should not repeat the inferred correct status") + _assert("[bgcolor=yellow]" in player.explanation_label.text, "explanation text should support highlighting") + player.explanation_label.free() + player.free() + + +func _test_collapsed_followup_review_expands() -> void: + var player = load("res://scripts/ui/question_player.gd").new() + var panel := PanelContainer.new() + var content := VBoxContainer.new() + panel.add_child(content) + player.choice_buttons.append(Button.new()) + player.choice_percent_labels.append(Label.new()) + player.choice_top_labels.append(RichTextLabel.new()) + player.choice_followup_panels.append(panel) + player.choice_followup_contents.append(content) + player.next_button = Button.new() + player.presented_choices = [{"id": "B", "text": "Wrong answer"}] + player.followup_states["B"] = { + "choice_id": "B", "choice_text": "Wrong answer", "prompt": "Why?", + "selected_option_text": "Finding", "was_correct": false, + "teaching_point": "Review the finding.", "resolved": true, "collapsed": true + } + player._render_followup_state("B") + var review_button := content.get_child(0) as Button + _assert(review_button != null, "collapsed follow-up should render a review button") + review_button.pressed.emit() + _assert(not bool(player.followup_states["B"].get("collapsed", true)), "review button should expand the resolved follow-up") + player.choice_buttons[0].free() + player.choice_percent_labels[0].free() + player.choice_top_labels[0].free() + player.next_button.free() + panel.free() + player.free() + + func _assert(condition: bool, message: String) -> void: if condition: return