feat(modal): implement support modal visibility logic based on localstorage #110
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @sunithvs, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience by introducing intelligent visibility logic for the SupportModal. Instead of appearing on every page load, the modal now leverages local storage to determine if it should be displayed, showing only on a user's initial visit and then respecting a 24-hour cooldown period after being closed. This change aims to reduce user fatigue and make the modal less intrusive for returning users. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a well-thought-out feature to improve user experience by controlling the visibility of the support modal using localStorage. The implementation is solid and the PR description is excellent. I've added a couple of suggestions to enhance the robustness of the localStorage handling and improve code maintainability by using constants and useCallback.
| 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; | ||
| } | ||
| }; |
There was a problem hiding this comment.
This function can be made more robust and maintainable.
- Data Validation: The code currently assumes
JSON.parsewill yield an object with a numerictimestamp. IflocalStoragecontains malformed data (e.g.,"{}"or{"timestamp": "abc"}),now - timestampwill beNaN. This causes the check to fail silently, incorrectly preventing the modal from showing. You should validate the parsed data's structure. - Hardcoded Values: The
localStoragekey 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 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); | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
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);
}
}
}, []);
Add localStorage expiry for SupportModal to improve UX
📋 Summary
Implements localStorage-based expiry functionality for the SupportModal to prevent it from showing on every page visit. The modal now shows only on first visit and then respects a 1-day cooldown period.
🎯 Problem Statement
The SupportModal was showing on every page visit after a 4-second delay, which could be intrusive for returning users. Users needed a way to dismiss the modal temporarily without it reappearing immediately on subsequent visits.
💡 Solution
Added localStorage-based expiry system that:
🔧 Technical Changes
Modified Files
www/components/modal/support-modal.tsxKey Implementation Details
localStorage Check Function
Timestamp Storage on Close
Updated Modal Logic
useEffectto use the new check functiononOpenChangeto use the new handler🧪 Testing
Test Cases
Manual Testing Steps
devb-support-modalentry🔒 Edge Cases Handled
📊 Impact
User Experience
Technical
🎨 Configuration
The expiry period can be easily adjusted by modifying:
Possible configurations:
60 * 60 * 10007 * 24 * 60 * 60 * 100030 * 24 * 60 * 60 * 1000🔮 Future Enhancements
✅ Checklist