Skip to content

feat(modal): implement support modal visibility logic based on localstorage #110

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.

Sign up for GitHub

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
sunithvs merged 1 commit into from
Sep 20, 2025
Merged

feat(modal): implement support modal visibility logic based on localstorage #110

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 40 additions & 2 deletions www/components/modal/support-modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,51 @@ export const SupportModal = ({ user }: { user: Profile | null }) => {
const [copied, setCopied] = useState(false);

useEffect(() => {
const checkShouldShowModal = () => {
try {
const modalData = localStorage.getItem('devb-support-modal');

if (!modalData) {
// First time visit - show modal
return true;
}

const { timestamp } = JSON.parse(modalData);
const oneDayInMs = 24 * 60 * 60 * 1000; // 1 day in milliseconds
const now = Date.now();

// Show modal if more than 1 day has passed
return (now - timestamp) > oneDayInMs;
} catch (error) {
// If there's any error with localStorage, show modal
console.warn('Error checking modal state:', error);
return true;
}
};
Comment on lines +23 to +43
Copy link

@gemini-code-assist gemini-code-assist bot Sep 20, 2025

Choose a reason for hiding this comment

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

high

This function can be made more robust and maintainable.

  1. Data Validation: The code currently assumes JSON.parse will yield an object with a numeric timestamp. If localStorage contains malformed data (e.g., "{}" or {"timestamp": "abc"}), now - timestamp will be NaN. This causes the check to fail silently, incorrectly preventing the modal from showing. You should validate the parsed data's structure.
  2. Hardcoded Values: The localStorage key is hardcoded. Defining it as a constant improves readability and makes future changes easier, especially since it's used in another function.
    const checkShouldShowModal = () => {
      const MODAL_STORAGE_KEY = 'devb-support-modal';
      const MODAL_COOLDOWN_MS = 24 * 60 * 60 * 1000; // 1 day in milliseconds

      try {
        const modalDataString = localStorage.getItem(MODAL_STORAGE_KEY);
        if (!modalDataString) {
          return true; // First time visit
        }

        const modalData = JSON.parse(modalDataString);
        if (typeof modalData?.timestamp !== 'number') {
          console.warn('Invalid modal data in localStorage. Resetting.');
          localStorage.removeItem(MODAL_STORAGE_KEY); // Clean up invalid data
          return true;
        }

        const now = Date.now();
        return (now - modalData.timestamp) > MODAL_COOLDOWN_MS;
      } catch (error) {
        console.warn('Error checking modal state:', error);
        return true;
      }
    };


const timer = setTimeout(() => {
setOpen(!user?.cached || false);
const shouldShow = checkShouldShowModal();
setOpen(shouldShow && (!user?.cached || false));
}, 4000);

return () => clearTimeout(timer);
}, [user]);

const handleModalClose = (isOpen: boolean) => {
setOpen(isOpen);
if (!isOpen) {
try {
// Save timestamp when modal is closed
const modalData = {
timestamp: Date.now(),
};
localStorage.setItem('devb-support-modal', JSON.stringify(modalData));
} catch (error) {
console.warn('Error saving modal state:', error);
}
}
};
Comment on lines +53 to +66
Copy link

@gemini-code-assist gemini-code-assist bot Sep 20, 2025

Choose a reason for hiding this comment

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

medium

This handler function is recreated on every render. While it may not cause performance issues here, it's a good practice to memoize handlers with useCallback to prevent unnecessary re-renders of child components. Additionally, the localStorage key is hardcoded. Using a constant (as suggested for checkShouldShowModal) would be better for maintainability.

  const handleModalClose = React.useCallback((isOpen: boolean) => {
    const MODAL_STORAGE_KEY = 'devb-support-modal';
    setOpen(isOpen);
    if (!isOpen) {
      try {
        // Save timestamp when modal is closed
        const modalData = {
          timestamp: Date.now(),
        };
        localStorage.setItem(MODAL_STORAGE_KEY, JSON.stringify(modalData));
      } catch (error) {
        console.warn('Error saving modal state:', error);
      }
    }
  }, []);


const copyToClipboard = () => {
navigator.clipboard
.writeText(DEVB_INVITE_LINK)
Expand All @@ -44,7 +82,7 @@ export const SupportModal = ({ user }: { user: Profile | null }) => {
};

return (
<Dialog open={open} onOpenChange={setOpen}>
<Dialog open={open} onOpenChange={handleModalClose}>
<DialogContent className="sm:max-w-[700px] p-0 overflow-hidden border-black border-1 border-b-4 rounded-3xl">
<div className="p-6 pt-8">
<div className="bg-[#B9FF66] inline-block px-4 py-2 rounded-lg mb-4">
Expand Down
Loading