Fix changelog display in About window #133

mkh-user · 2025-11-20T10:56:18Z

Type of Change

Bug fix

Description

Fixes #132 with use visibility_changed instead of about_to_popup signal.

Testing

Works fine.

Impact

A small freeze during conversion, this is the same as the original implementation (less than 0.3ms) and is moved to the opening time of the window instead of app startup. (See #83 for more information)

Additional Information

Nothing

Checklist

My code adheres to the coding and style guidelines of the project.
I have performed a self-review of my own code.
I have commented my code, particularly in hard-to-understand areas.
I have made corresponding changes to the documentation.
My changes generate no new warnings
Changes were added to CHANGELOG

Summary by CodeRabbit

Bug Fixes
- Resolved issue where Changelog was not displaying in the editor.
Documentation
- Updated CHANGELOG with unreleased fixes section.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-11-20T10:56:27Z

📝 Walkthrough

Walkthrough

Changes fix the changelog display issue in the About window by switching the trigger signal from about_to_popup (which was not emitting) to visibility_changed (which fires reliably). Updated CHANGELOG.md to document the fix and renamed the corresponding callback function.

Changes

Cohort / File(s)	Summary
Documentation Update `CHANGELOG.md`	Added [unreleased] section with Fixed subsection documenting the resolved changelog display issue (`#133`)
Signal Connection Update `core/main.tscn`	Changed signal connection for Change Log tab from `about_to_popup` to `visibility_changed`, updating the handler reference accordingly
Handler Rename `core/scripts/change_log.gd`	Renamed callback function from `_on_about_about_to_popup()` to `_on_about_visibility_changed()` to match new signal connection

Sequence Diagram(s)

sequenceDiagram
    rect rgb(230, 240, 250)
    Note over About: Old Flow (Broken)
    About->>About: about_to_popup signal<br/>(not emitted)
    About--xchange_log.gd: _on_about_about_to_popup()<br/>(never called)
    end
    
    rect rgb(240, 250, 240)
    Note over About: New Flow (Fixed)
    User->>About: Open About window
    About->>About: visibility_changed signal
    About->>change_log.gd: _on_about_visibility_changed()
    change_log.gd->>change_log.gd: Read CHANGELOG.md
    change_log.gd->>change_log.gd: Convert markdown to BBCode
    change_log.gd->>TabContainer: Display in Changelog tab
    end

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	The PR successfully addresses issue #132 by replacing the non-emitted about_to_popup signal with visibility_changed signal to fix changelog display.
Out of Scope Changes check	✅ Passed	All changes directly address the linked issue #132 with no unrelated modifications to the codebase.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check	✅ Passed	The title accurately summarizes the main change: fixing changelog display in the About window by switching from an unworking signal to a working one.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch 132-changelog-is-not-displayed

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

core/scripts/change_log.gd (1)
62-65: Consider adding a visibility check to avoid redundant processing.

The visibility_changed signal fires when the About window is both shown and hidden. Currently, the markdown conversion runs in both cases, which wastes CPU cycles when closing the window (though the impact is minimal at ≤0.3ms).

Apply this diff to skip conversion when the window is closing:
 func _on_about_visibility_changed() -> void:
+	# Only convert when the About window becomes visible, not when it hides
+	var about_window = get_parent().get_parent()  # Navigate to About window
+	if not about_window.visible:
+		return
 	var markdown_text := FileAccess.get_file_as_string(S.globalize_path("res://CHANGELOG.md"))
 	var bbcode_text := _convert_text_from_markdown_to_bbcode_style(markdown_text)
 	text = bbcode_text

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4685dc0 and c0e4e8a.

📒 Files selected for processing (3)

CHANGELOG.md (1 hunks)
core/main.tscn (1 hunks)
core/scripts/change_log.gd (1 hunks)

🔇 Additional comments (2)

CHANGELOG.md (1)

8-12: LGTM! Changelog entry correctly documents the fix.

The unreleased section and Fixed entry follow the established format and properly reference PR #133.

core/main.tscn (1)

500-500: Correct fix: visibility_changed is the appropriate signal for Window nodes.

The switch from about_to_popup to visibility_changed properly addresses the root cause. The about_to_popup signal is specific to Popup nodes and isn't available on Window nodes (which About is, as defined on line 402). The visibility_changed signal correctly fires when the About window's visibility changes.

core/scripts/change_log.gd

mkh-user added 3 commits November 20, 2025 13:48

Fix #132 Changelog display emission

6d9b46b

Merge branch 'Main' into 132-changelog-is-not-displayed

9c325e7

Update CHANGELOG

c0e4e8a

mkh-user linked an issue Nov 20, 2025 that may be closed by this pull request

Changelog is not displayed #132

Closed

2 tasks

github-project-automation bot added this to Release: Text Forge 1.0 Nov 20, 2025

github-project-automation bot moved this to Needs Review in Release: Text Forge 1.0 Nov 20, 2025

mkh-user self-assigned this Nov 20, 2025

coderabbitai bot changed the title ~~@coderabbit autotitle~~ Fix changelog display in About window autotitle Nov 20, 2025

mkh-user changed the title ~~Fix changelog display in About window autotitle~~ Fix changelog display in About window Nov 20, 2025

mkh-user added this to the Text Forge 0.2 milestone Nov 20, 2025

mkh-user moved this from Needs Review to In Progress in Release: Text Forge 1.0 Nov 20, 2025

coderabbitai bot reviewed Nov 20, 2025

View reviewed changes

core/scripts/change_log.gd Show resolved Hide resolved

mkh-user added the bug A confirmed issue causing incorrect behavior or unexpected results label Nov 20, 2025

mkh-user merged commit 9a9c28a into Main Nov 20, 2025
2 checks passed

mkh-user deleted the 132-changelog-is-not-displayed branch November 20, 2025 11:04

github-project-automation bot moved this from In Progress to Completed in Release: Text Forge 1.0 Nov 20, 2025

coderabbitai bot mentioned this pull request Dec 7, 2025

Release v0.2-rc2 and update About section styling #150

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix changelog display in About window #133

Fix changelog display in About window #133

Uh oh!

mkh-user commented Nov 20, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Nov 20, 2025 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Fix changelog display in About window #133

Fix changelog display in About window #133

Uh oh!

Conversation

mkh-user commented Nov 20, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Type of Change

Description

Testing

Impact

Additional Information

Checklist

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mkh-user commented Nov 20, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Nov 20, 2025 •

edited

Loading