Skip to content

fix: fullscreen action handling for create mode #756

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 3, 2025

Conversation

vcashwin
Copy link
Contributor

@vcashwin vcashwin commented Jun 3, 2025

PR Type

Bug fix


Description

  • Close overlay on fullscreen toggle

  • Import and use overlay store hook


Changes walkthrough 📝

Relevant files
Bug fix
MainContent.tsx
Add overlay closure in fullscreen handler                               

apps/app/components/welcome/featured/MainContent.tsx

  • Added import for useOverlayStore
  • Destructured setIsOverlayOpen from overlay store
  • Wrapped fullscreen button handler to close overlay first
  • +6/-1     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @vcashwin vcashwin temporarily deployed to Preview - e2e June 3, 2025 05:01 — with GitHub Actions Inactive
    Copy link

    vercel bot commented Jun 3, 2025

    The latest updates on your projects. Learn more about Vercel for Git ↗︎

    Name Status Preview Comments Updated (UTC)
    pipelines-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 3, 2025 5:03am

    Copy link

    github-actions bot commented Jun 3, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Import Path Consistency

    The new import of useOverlayStore uses an absolute alias path (@/hooks/useOverlayStore) while other hooks in this file use relative paths, which may lead to inconsistencies or module resolution errors.

    import { useOverlayStore } from "@/hooks/useOverlayStore";
    Missing Tests

    There are no tests added or updated to validate the new behavior of closing the overlay on fullscreen toggle, leaving this change unverified in the test suite.

    onClick={() => {
      setIsOverlayOpen(false);
      toggleFullscreen();
    }}

    Copy link

    github-actions bot commented Jun 3, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Conditionally restore overlay state

    Restore the overlay state when exiting fullscreen by checking the current
    isFullscreen value and toggling the overlay accordingly. This ensures the overlay
    reopens after leaving fullscreen.

    apps/app/components/welcome/featured/MainContent.tsx [48-51]

     onClick={() => {
    -  setIsOverlayOpen(false);
    +  const entering = !isFullscreen;
       toggleFullscreen();
    +  setIsOverlayOpen(!entering);
     }}
    Suggestion importance[1-10]: 7

    __

    Why: The suggestion ensures the overlay reopens after exiting fullscreen by correctly toggling based on isFullscreen, improving user experience.

    Medium

    @vcashwin vcashwin merged commit 7de1a5d into main Jun 3, 2025
    6 checks passed
    @vcashwin vcashwin deleted the ashwin/fix-fullscreen-issue branch June 3, 2025 07:41
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant