Skip to content

Conversation

@kunalbansal23cs227
Copy link

@kunalbansal23cs227 kunalbansal23cs227 commented Nov 11, 2025

Closes #623

Addresses the flickering issue in the InfoDialog component, where the dialog momentarily switches to the blue variant upon closing an error dialog.

This update begins refactoring the dialog state handling logic to ensure consistent visual feedback and transition behaviour.

Testing Videos before and after changes :

Before :

before_update.mov

After:

after_update.mov

Summary by CodeRabbit

  • Bug Fixes
    • Fixed dialog variant state persistence. Dialogs now correctly maintain their previously configured variant type when hidden, instead of being reset to the default variant.

@github-actions github-actions bot added UI bug Something isn't working labels Nov 11, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 11, 2025

Walkthrough

Modified the hideInfoDialog reducer to preserve the dialog's existing variant instead of resetting it to 'info'. The variant now defaults to 'info' only when no prior variant exists, using the nullish coalescing assignment operator.

Changes

Cohort / File(s) Change Summary
Info Dialog Variant Preservation
frontend/src/features/infoDialogSlice.ts
Modified hideInfoDialog reducer to use state.variant ||= 'info' instead of state.variant = 'info', preserving the current variant throughout dialog closure animations

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Single-line logic change in one reducer function
  • Clear intent: prevent variant flickering during dialog closure
  • Addresses the visual flicker bug directly without side effects

Possibly related PRs

Poem

A fleeting flicker plagued the screen so blue,
When errors faded, shifting hue to hue—
Now dialogs hold their crimson shades with pride,
No more the stuttering, the wild color glide! 🐰✨

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.
Title check ✅ Passed The title clearly and specifically describes the main fix: preventing InfoDialog from flickering to blue variant when closing, which directly matches the core issue and changeset.
Linked Issues check ✅ Passed The changes address the linked issue #623 by modifying the hideInfoDialog logic to preserve the variant state, preventing the unwanted flicker to blue variant during close animation.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the InfoDialog flicker issue in infoDialogSlice.ts with no unrelated modifications present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

Copy link
Contributor

@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: 0

🧹 Nitpick comments (2)
frontend/src/features/infoDialogSlice.ts (2)

35-35: Prefer nullish coalescing (??) over logical OR (||).

While || works for this use case, ?? is more precise in TypeScript as it only defaults when the value is null or undefined, not when it's any falsy value. Given that InfoDialogVariant is likely a union of non-empty string literals, this distinction matters for type safety.

Apply this diff:

-      state.variant = state.variant || 'info';
+      state.variant = state.variant ?? 'info';

31-37: Consider deferring state cleanup until after the close animation completes.

The current approach clears title and message immediately while preserving variant to prevent the flicker. This creates inconsistent state handling—some fields are cleared while others are preserved. While this fixes the immediate symptom, a more robust architectural solution would be to preserve the entire dialog state until the closing animation finishes, then clear everything. This could be achieved by:

  • Using an animation callback (e.g., onAnimationEnd) to dispatch state cleanup
  • Introducing a separate "closing" state distinct from "closed"
  • Letting the component handle state cleanup after animation

The current fix works but may be fragile if other fields (title, message) are also rendered during the close animation.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cec1303 and 7104d90.

📒 Files selected for processing (1)
  • frontend/src/features/infoDialogSlice.ts (1 hunks)

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

Labels

bug Something isn't working UI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: InfoDialog Flickers to blue variant when closing error dialog.

2 participants