Skip to content

feat: resolve the issue of the close button continuously displaying after enlarging the image#459

Open
wuyiping0628 wants to merge 1 commit intodevfrom
wyp/show-image-0319
Open

feat: resolve the issue of the close button continuously displaying after enlarging the image#459
wuyiping0628 wants to merge 1 commit intodevfrom
wyp/show-image-0319

Conversation

@wuyiping0628
Copy link
Collaborator

@wuyiping0628 wuyiping0628 commented Mar 19, 2026

…fter enlarging the image

PR

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our Commit Message Guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: #452

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Summary by CodeRabbit

  • Bug Fixes
    • Improved close button state management in the preview modal to ensure proper display behavior and cleanup.

@github-actions github-actions bot added the enhancement New feature or request label Mar 19, 2026
@coderabbitai
Copy link

coderabbitai bot commented Mar 19, 2026

Walkthrough

The preview modal's close button is refactored from a locally-scoped variable to a persistent class field, enabling explicit visibility management (display: block/none) across the modal's lifecycle—initialization, display, hiding, and destruction phases.

Changes

Cohort / File(s) Summary
Close Button Lifecycle Management
packages/fluent-editor/src/modules/custom-image/preview/preview-modal.ts
Added persistent closeBtn class field and explicit display toggling in show() and hide() methods; added cleanup in destroy() method to remove and nullify the button reference.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A button once lost in the local scope's embrace,
Now persists through the modal's entire life-long race—
Show, hide, destroy—each phase knows its state,
Lifecycle management, crisp and first-rate! ✨🔘

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The PR title accurately describes the main change: fixing an issue where the close button continuously displays after enlarging an image.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch wyp/show-image-0319
📝 Coding Plan
  • Generate coding plan for human review comments

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.

Tip

You can validate your CodeRabbit configuration file in your editor.

If your editor has YAML language server, you can enable auto-completion and validation by adding # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json at the top of your CodeRabbit configuration file.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/fluent-editor/src/modules/custom-image/preview/preview-modal.ts (1)

68-86: Consider using textContent instead of innerHTML.

While the hardcoded × character isn't a security risk, using textContent is a safer pattern and avoids static analysis warnings.

🔧 Proposed fix
    this.closeBtn = document.createElement('button')
    this.closeBtn.className = 'tiny-editor-image-preview-close'
-   this.closeBtn.innerHTML = '×'
+   this.closeBtn.textContent = '×'
    this.closeBtn.style.cssText = `
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/fluent-editor/src/modules/custom-image/preview/preview-modal.ts`
around lines 68 - 86, Replace the use of innerHTML on the modal close button
with textContent to avoid unnecessary HTML parsing/static analysis warnings:
locate the this.closeBtn setup in preview-modal.ts where the button is created
(references: this.closeBtn, class 'tiny-editor-image-preview-close') and change
this.closeBtn.innerHTML = '×' to use this.closeBtn.textContent = '×'; keep the
rest of the setup and the click handler (this.hide()) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/fluent-editor/src/modules/custom-image/preview/preview-modal.ts`:
- Around line 230-232: The destroy() method currently nulls instance fields but
does not reset the singleton reference, leaving globalPreviewModal pointing to a
dead instance; update destroy() in the PreviewModal class (the instance returned
by getImagePreviewModal()) to also set the module-level globalPreviewModal
variable to null so subsequent calls to getImagePreviewModal() create a fresh,
usable instance and avoid show() returning early on a destroyed object.

---

Nitpick comments:
In `@packages/fluent-editor/src/modules/custom-image/preview/preview-modal.ts`:
- Around line 68-86: Replace the use of innerHTML on the modal close button with
textContent to avoid unnecessary HTML parsing/static analysis warnings: locate
the this.closeBtn setup in preview-modal.ts where the button is created
(references: this.closeBtn, class 'tiny-editor-image-preview-close') and change
this.closeBtn.innerHTML = '×' to use this.closeBtn.textContent = '×'; keep the
rest of the setup and the click handler (this.hide()) unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e0b702aa-9bec-4e56-8f08-fcd3b7032e34

📥 Commits

Reviewing files that changed from the base of the PR and between 270a8cd and 58e120d.

📒 Files selected for processing (1)
  • packages/fluent-editor/src/modules/custom-image/preview/preview-modal.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant