hz/step3
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Simplify resolved wrong-answer explanation cards per design feedback

Browse Source 
Drop the redundant header row (duplicate percentage badge + collapse
chevron), the "Why this is appropriate/less appropriate:"/"Why this
is incorrect:" heading labels, and the nested sub-box - now that
these cards live inside their own choice's colored border, all of
that was repeating what the color and position already say. Both the
selected-wrong-choice resolved follow-up and the never-selected
distractor cards now just show the plain explanation sentence, with
the leading "Correct."/"Incorrect." verdict word stripped too.

Removing the toggle chevron removes the only way to reach the
collapse/expand behavior, so that machinery (_collapse_resolved_followups,
_toggle_followup_state, the "collapsed" state field) comes out too
rather than linger unreachable.
This commit is contained in:
hz
2026-07-23 04:19:10 -05:00
parent d69a1fd0e3
commit b3a8dec379
2 changed files with 23 additions and 91 deletions
Download Patch File Download Diff File Expand all files Collapse all files
scripts/ui/question_player.gd
+13 -72
View File
@@ -1346,7 +1346,6 @@ func _submit_or_next() -> void:
return return
var question_id_for_attempt := str(BlockSessionService.get_current_question().get("id", "")) var question_id_for_attempt := str(BlockSessionService.get_current_question().get("id", ""))
if BlockSessionService.session_kind in ["core", "bookmark", "review"]: if BlockSessionService.session_kind in ["core", "bookmark", "review"]:
_collapse_resolved_followups()
var attempted_choice := selected_choice var attempted_choice := selected_choice
if BlockSessionService.session_kind == "core": if BlockSessionService.session_kind == "core":
QuestionBankService.record_first_attempt( QuestionBankService.record_first_attempt(
@@ -1558,7 +1557,7 @@ func _render_teaching_cards() -> void:
if takeaway == "": if takeaway == "":
var correct_id := str(question.get("correct_choice_id", "")) var correct_id := str(question.get("correct_choice_id", ""))
var explanations: Dictionary = question.get("choice_explanations", {}) var explanations: Dictionary = question.get("choice_explanations", {})
takeaway = _strip_correct_prefix(str(explanations.get(correct_id, "")).strip_edges()) takeaway = _strip_verdict_prefix(str(explanations.get(correct_id, "")).strip_edges())
var pearl := str(question.get("usmle_pearl", "")).strip_edges() var pearl := str(question.get("usmle_pearl", "")).strip_edges()
if takeaway != "": if takeaway != "":
_add_teaching_card("KEY TAKEAWAY", takeaway, Brand.SKY_BLUE) _add_teaching_card("KEY TAKEAWAY", takeaway, Brand.SKY_BLUE)
@@ -1686,16 +1685,11 @@ func _render_unselected_distractor_explanations() -> void:
continue continue
_clear_followup_content(index) _clear_followup_content(index)
var content := choice_followup_contents[index] var content := choice_followup_contents[index]
var body_content := _add_followup_body(content, false)
var heading := Label.new()
heading.text = "Why this is incorrect:"
_set_scaled_label(heading, 14, Color("fb5f6d"))
body_content.add_child(heading)
var explanation := Label.new() var explanation := Label.new()
explanation.text = _distractor_explanation_text(choice_id) explanation.text = _strip_verdict_prefix(_distractor_explanation_text(choice_id))
explanation.autowrap_mode = TextServer.AUTOWRAP_WORD_SMART explanation.autowrap_mode = TextServer.AUTOWRAP_WORD_SMART
_set_scaled_label(explanation, 14, Brand.text_color(_followup_is_dark())) _set_scaled_label(explanation, 14, Brand.text_color(_followup_is_dark()))
body_content.add_child(explanation) content.add_child(explanation)
choice_followup_panels[index].add_theme_stylebox_override( choice_followup_panels[index].add_theme_stylebox_override(
"panel", "panel",
_attached_panel_box(_followup_is_dark(), Color("fb7185")) _attached_panel_box(_followup_is_dark(), Color("fb7185"))
@@ -1708,16 +1702,16 @@ func _distractor_explanation_text(choice_id: String) -> String:
var question := BlockSessionService.get_current_question() var question := BlockSessionService.get_current_question()
var explanations: Dictionary = question.get("choice_explanations", {}) var explanations: Dictionary = question.get("choice_explanations", {})
var explanation := str(explanations.get(choice_id, "")).strip_edges() var explanation := str(explanations.get(choice_id, "")).strip_edges()
if explanation.begins_with("Correct. "):
explanation = explanation.substr(9)
if explanation != "": if explanation != "":
return explanation return explanation
return "Review the stem finding that makes this option less appropriate." return "Review the stem finding that makes this option less appropriate."
func _strip_correct_prefix(value: String) -> String: func _strip_verdict_prefix(value: String) -> String:
if value.begins_with("Correct. "): if value.begins_with("Correct. "):
return value.substr(9) return value.substr(9)
if value.begins_with("Incorrect. "):
return value.substr(11)
return value return value
@@ -3162,7 +3156,6 @@ func _build_followup_panel(_index: int, choice_id: String) -> void:
"selected_option_text": "", "selected_option_text": "",
"was_correct": false, "was_correct": false,
"resolved": false, "resolved": false,
"collapsed": false,
"has_authored_choices": bool(followup.get("has_authored_choices", false)), "has_authored_choices": bool(followup.get("has_authored_choices", false)),
"is_main_correct": false "is_main_correct": false
} }
@@ -3177,7 +3170,7 @@ func _append_followup_attempt(choice_id: String) -> void:
func _render_active_followup(index: int, state: Dictionary) -> void: func _render_active_followup(index: int, state: Dictionary) -> void:
var content := choice_followup_contents[index] var content := choice_followup_contents[index]
_build_followup_header(content, state, true, false) _build_followup_header(content, state)
var body_content := _add_followup_body(content, false) var body_content := _add_followup_body(content, false)
var prompt := Label.new() var prompt := Label.new()
@@ -3235,7 +3228,6 @@ func _resolve_followup(_followup: Dictionary, selected_button: Button, _style_se
state["selected_option_text"] = selected_option_text state["selected_option_text"] = selected_option_text
state["was_correct"] = is_correct state["was_correct"] = is_correct
state["resolved"] = true state["resolved"] = true
state["collapsed"] = false
followup_states[resolved_choice_id] = state followup_states[resolved_choice_id] = state
_render_followup_state(resolved_choice_id) _render_followup_state(resolved_choice_id)
@@ -3251,28 +3243,6 @@ func _unlock_after_followup() -> void:
_apply_saved_choice_state() _apply_saved_choice_state()
func _collapse_resolved_followups() -> void:
for choice_id in followup_states.keys():
var state: Dictionary = followup_states[choice_id]
if bool(state.get("resolved", false)):
state["collapsed"] = true
followup_states[choice_id] = state
_render_followup_state(choice_id)
_apply_saved_choice_state()
func _toggle_followup_state(choice_id: String) -> void:
if not followup_states.has(choice_id) or followup_required:
return
var state: Dictionary = followup_states[choice_id]
if not bool(state.get("resolved", false)):
return
state["collapsed"] = not bool(state.get("collapsed", false))
followup_states[choice_id] = state
_render_followup_state(choice_id)
_apply_saved_choice_state()
func _render_followup_state(choice_id: String) -> void: func _render_followup_state(choice_id: String) -> void:
if not followup_states.has(choice_id): if not followup_states.has(choice_id):
return return
@@ -3284,8 +3254,6 @@ func _render_followup_state(choice_id: String) -> void:
_style_followup_card(index, state) _style_followup_card(index, state)
if not bool(state.get("resolved", false)): if not bool(state.get("resolved", false)):
_render_active_followup(index, state) _render_active_followup(index, state)
elif bool(state.get("collapsed", false)):
_build_followup_header(choice_followup_contents[index], state, false, true)
else: else:
_build_resolved_followup(index, state) _build_resolved_followup(index, state)
choice_followup_panels[index].visible = true choice_followup_panels[index].visible = true
@@ -3294,22 +3262,14 @@ func _render_followup_state(choice_id: String) -> void:
func _build_resolved_followup(index: int, state: Dictionary) -> void: func _build_resolved_followup(index: int, state: Dictionary) -> void:
var content := choice_followup_contents[index] var content := choice_followup_contents[index]
_build_followup_header(content, state, true, true)
var is_main_correct := bool(state.get("is_main_correct", false))
var body_content := _add_followup_body(content, is_main_correct)
var heading := Label.new()
heading.text = "Why this is appropriate:" if is_main_correct else "Why this is less appropriate:"
_set_scaled_label(heading, 14, Brand.SUCCESS if is_main_correct else Color("fb5f6d"))
body_content.add_child(heading)
var explanation := Label.new() var explanation := Label.new()
explanation.text = _choice_review_explanation(str(state.get("choice_id", "")), state) explanation.text = _strip_verdict_prefix(_choice_review_explanation(str(state.get("choice_id", "")), state))
explanation.autowrap_mode = TextServer.AUTOWRAP_WORD_SMART explanation.autowrap_mode = TextServer.AUTOWRAP_WORD_SMART
_set_scaled_label(explanation, 14, Brand.text_color(_followup_is_dark())) _set_scaled_label(explanation, 14, Brand.text_color(_followup_is_dark()))
body_content.add_child(explanation) content.add_child(explanation)
func _build_followup_header(content: VBoxContainer, state: Dictionary, expanded: bool, interactive: bool) -> void:
func _build_followup_header(content: VBoxContainer, state: Dictionary) -> void:
var dark := _followup_is_dark() var dark := _followup_is_dark()
var is_main_correct := bool(state.get("is_main_correct", false)) var is_main_correct := bool(state.get("is_main_correct", false))
var accent := Brand.SUCCESS if is_main_correct else Color("fb5f6d") var accent := Brand.SUCCESS if is_main_correct else Color("fb5f6d")
@@ -3317,9 +3277,8 @@ func _build_followup_header(content: VBoxContainer, state: Dictionary, expanded:
row.add_theme_constant_override("separation", 10) row.add_theme_constant_override("separation", 10)
content.add_child(row) content.add_child(row)
# The choice button above this panel already shows the letter badge, choice # The choice button above this panel already shows the letter badge and
# text, and percentage, so the header only needs a spacer plus the collapse # choice text, so the header only needs a spacer plus the percentage badge.
# toggle - repeating "Review: B. ..." and a status icon here was redundant.
var spacer := Control.new() var spacer := Control.new()
spacer.size_flags_horizontal = Control.SIZE_EXPAND_FILL spacer.size_flags_horizontal = Control.SIZE_EXPAND_FILL
row.add_child(spacer) row.add_child(spacer)
@@ -3334,17 +3293,6 @@ func _build_followup_header(content: VBoxContainer, state: Dictionary, expanded:
_set_scaled_label(percentage_label, 14, accent) _set_scaled_label(percentage_label, 14, accent)
row.add_child(percentage_label) row.add_child(percentage_label)
if interactive:
var toggle := Button.new()
toggle.name = "FollowupToggle"
toggle.flat = true
toggle.tooltip_text = "Collapse review" if expanded else "Expand review"
toggle.icon = preload("res://assets/icons/lucide/chevron-down.svg") if expanded else preload("res://assets/icons/lucide/chevron-right.svg")
toggle.custom_minimum_size = Vector2(34, 30)
toggle.add_theme_color_override("icon_normal_color", Brand.text_color(dark))
toggle.pressed.connect(_toggle_followup_state.bind(str(state.get("choice_id", ""))))
row.add_child(toggle)
func _add_followup_body(content: VBoxContainer, is_correct: bool) -> VBoxContainer: func _add_followup_body(content: VBoxContainer, is_correct: bool) -> VBoxContainer:
var body := PanelContainer.new() var body := PanelContainer.new()
@@ -3390,8 +3338,6 @@ func _choice_review_explanation(choice_id: String, state: Dictionary) -> String:
var question := BlockSessionService.get_current_question() var question := BlockSessionService.get_current_question()
var explanations: Dictionary = question.get("choice_explanations", {}) var explanations: Dictionary = question.get("choice_explanations", {})
var explanation := str(explanations.get(choice_id, "")).strip_edges() var explanation := str(explanations.get(choice_id, "")).strip_edges()
if explanation.begins_with("Correct. "):
explanation = explanation.substr(9)
if explanation != "": if explanation != "":
return explanation return explanation
var teaching_point := str(state.get("teaching_point", "")).strip_edges() var teaching_point := str(state.get("teaching_point", "")).strip_edges()
@@ -3403,11 +3349,6 @@ func _choice_review_explanation(choice_id: String, state: Dictionary) -> String:
func _ensure_answer_review_followups() -> void: func _ensure_answer_review_followups() -> void:
for attempted_id in followup_states.keys():
var attempted_state: Dictionary = followup_states[attempted_id]
if bool(attempted_state.get("resolved", false)):
attempted_state["collapsed"] = true
followup_states[attempted_id] = attempted_state
for choice_id in followup_attempt_order: for choice_id in followup_attempt_order:
_render_followup_state(str(choice_id)) _render_followup_state(str(choice_id))
tests/post_answer_review_test.gd
+10 -19
View File
@@ -13,10 +13,9 @@ func _initialize() -> void:
_test_eliminated_choice_uses_rich_text_strikeout() _test_eliminated_choice_uses_rich_text_strikeout()
_test_eliminated_choice_that_is_also_locked_wrong_still_strikes_through() _test_eliminated_choice_that_is_also_locked_wrong_still_strikes_through()
_test_explanation_highlighting_has_no_status_heading() _test_explanation_highlighting_has_no_status_heading()
_test_collapsed_followup_review_expands() _test_resolved_followup_has_no_header_or_verdict_prefix()
_test_review_content_normalization() _test_review_content_normalization()
_test_followup_attempt_order_is_stable() _test_followup_attempt_order_is_stable()
_test_answer_review_collapses_nested_wrong_followups()
_test_review_followup_is_optional_and_single_attempt() _test_review_followup_is_optional_and_single_attempt()
_test_question_failure_retry_classification() _test_question_failure_retry_classification()
_test_transient_question_failure_keeps_player_waiting() _test_transient_question_failure_keeps_player_waiting()
@@ -178,7 +177,7 @@ func _test_explanation_highlighting_has_no_status_heading() -> void:
player.free() player.free()
func _test_collapsed_followup_review_expands() -> void: func _test_resolved_followup_has_no_header_or_verdict_prefix() -> void:
var player = load("res://scripts/ui/question_player.gd").new() var player = load("res://scripts/ui/question_player.gd").new()
var panel := PanelContainer.new() var panel := PanelContainer.new()
var content := VBoxContainer.new() var content := VBoxContainer.new()
@@ -193,14 +192,16 @@ func _test_collapsed_followup_review_expands() -> void:
player.followup_states["B"] = { player.followup_states["B"] = {
"choice_id": "B", "choice_text": "Wrong answer", "prompt": "Why?", "choice_id": "B", "choice_text": "Wrong answer", "prompt": "Why?",
"selected_option_text": "Finding", "was_correct": false, "selected_option_text": "Finding", "was_correct": false,
"teaching_point": "Review the finding.", "resolved": true, "collapsed": true "teaching_point": "Incorrect. Review the finding.", "resolved": true
} }
player._render_followup_state("B") player._render_followup_state("B")
var review_button := content.find_child("FollowupToggle", true, false) as Button _assert(content.find_child("FollowupToggle", true, false) == null, "a resolved follow-up should not render a collapse toggle")
_assert(review_button != null, "collapsed follow-up should render a toggle button") var explanation_text := ""
if review_button != null: for child in content.get_children():
review_button.pressed.emit() if child is Label:
_assert(not bool(player.followup_states["B"].get("collapsed", true)), "review button should expand the resolved follow-up") explanation_text += child.text
_assert(not explanation_text.contains("Why this is"), "a resolved follow-up should not repeat a 'Why this is...' heading")
_assert(not explanation_text.begins_with("Incorrect."), "a resolved follow-up explanation should not repeat the verdict prefix")
player.choice_buttons[0].free() player.choice_buttons[0].free()
player.choice_percent_labels[0].free() player.choice_percent_labels[0].free()
player.choice_top_labels[0].free() player.choice_top_labels[0].free()
@@ -248,16 +249,6 @@ func _test_followup_attempt_order_is_stable() -> void:
player.free() player.free()
func _test_answer_review_collapses_nested_wrong_followups() -> void:
var player = load("res://scripts/ui/question_player.gd").new()
player.followup_states["C"] = {"resolved": true, "collapsed": false, "is_main_correct": false}
player.followup_attempt_order = ["C"]
player._ensure_answer_review_followups()
_assert(bool(player.followup_states["C"].get("collapsed", false)), "resolved distractor remediation should collapse for final review")
_assert(not player.followup_states.has("B"), "the correct answer should not become a distractor-remediation card")
player.free()
func _test_review_followup_is_optional_and_single_attempt() -> void: func _test_review_followup_is_optional_and_single_attempt() -> void:
var session = root.get_node("BlockSessionService") var session = root.get_node("BlockSessionService")
var prior_questions: Array = session.active_questions.duplicate(true) var prior_questions: Array = session.active_questions.duplicate(true)