feat: Persist markdown text to local storage by ndrvndr · Pull Request #7 · dannycahyo/air-deck

ndrvndr · 2025-12-05T04:03:16Z

Automatically save the user's work in the editor to the browser's local storage to prevent data loss on refresh.

Features:

Auto-save markdown text to localStorage
Restore text on page load
Prevents data loss on refresh

Technical implementation:

Hydration-safe storage initialization using isLoaded state
useEffect hook to load from 'airdeck-markdown' on mount
useEffect hook to sync state changes to storage
Prevents overwriting storage with initial empty state

Components:

PresentationContext.tsx: Added persistence logic

Automatically save the user's work in the editor to the browser's local storage to prevent data loss on refresh. Features: - Auto-save markdown text to `localStorage` - Restore text on page load - Prevents data loss on refresh Technical implementation: - Hydration-safe storage initialization using `isLoaded` state - `useEffect` hook to load from 'airdeck-markdown' on mount - `useEffect` hook to sync state changes to storage - Prevents overwriting storage with initial empty state Components: - PresentationContext.tsx: Added persistence logic

ndrvndr · 2025-12-06T18:28:34Z

Hi @dannycahyo , I'd like to follow up on the PR I submitted. I added a new feature that was previously mentioned as a future scope. If you have a moment, I’d really appreciate it if you could review it. I’d be happy to continue contributing to the project. Thank you!

Copilot

Pull request overview

This PR adds automatic persistence of markdown content to the browser's localStorage to prevent data loss on page refreshes.

Key Changes:

Added localStorage-based auto-save and restore functionality using React hooks
Implemented isLoaded state to prevent overwriting storage during hydration
Minor code formatting improvements (trailing commas, line breaks)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-09T15:13:57Z

app/contexts/PresentationContext.tsx

+    const saved = localStorage.getItem('airdeck-markdown');
+    if (saved) {
+      setMarkdownText(saved);


The localStorage access in this useEffect hook will cause SSR hydration errors since this application uses server-side rendering (ssr: true in react-router.config.ts). The localStorage object is not available on the server.

Recommendation: Add a check for the browser environment before accessing localStorage:

useEffect(() => { if (typeof window !== 'undefined') { const saved = localStorage.getItem('airdeck-markdown'); if (saved) { setMarkdownText(saved); } } setIsLoaded(true); }, []);

Suggested change

const saved = localStorage.getItem('airdeck-markdown');

if (saved) {

setMarkdownText(saved);

if (typeof window !== 'undefined') {

const saved = localStorage.getItem('airdeck-markdown');

if (saved) {

setMarkdownText(saved);

}

Copilot · 2025-12-09T15:13:58Z

app/contexts/PresentationContext.tsx

+    const saved = localStorage.getItem('airdeck-markdown');
+    if (saved) {
+      setMarkdownText(saved);
+    }
+    setIsLoaded(true);
+  }, []);
+
+  // Save to local storage when markdownText changes, but only after initial load
+  useEffect(() => {
+    if (isLoaded) {
+      localStorage.setItem('airdeck-markdown', markdownText);


Similar to the loading effect, this localStorage access will fail during SSR. Additionally, localStorage operations can throw exceptions in some browsers (e.g., when storage is full or in private browsing mode).

Recommendation: Add environment check and error handling:

useEffect(() => { if (isLoaded && typeof window !== 'undefined') { try { localStorage.setItem('airdeck-markdown', markdownText); } catch (error) { console.error('Failed to save to localStorage:', error); } } }, [markdownText, isLoaded]);

Suggested change

const saved = localStorage.getItem('airdeck-markdown');

if (saved) {

setMarkdownText(saved);

}

setIsLoaded(true);

}, []);

// Save to local storage when markdownText changes, but only after initial load

useEffect(() => {

if (isLoaded) {

localStorage.setItem('airdeck-markdown', markdownText);

if (typeof window !== 'undefined') {

try {

const saved = localStorage.getItem('airdeck-markdown');

if (saved) {

setMarkdownText(saved);

}

} catch (error) {

console.error('Failed to load from localStorage:', error);

}

}

setIsLoaded(true);

}, []);

// Save to local storage when markdownText changes, but only after initial load

useEffect(() => {

if (isLoaded && typeof window !== 'undefined') {

try {

localStorage.setItem('airdeck-markdown', markdownText);

} catch (error) {

console.error('Failed to save to localStorage:', error);

}

Copilot · 2025-12-09T15:13:58Z

app/contexts/PresentationContext.tsx

+    }
+    setIsLoaded(true);
+  }, []);
+


The initialMarkdown prop is ignored once data exists in localStorage. This could cause confusion if the prop is updated, as the stored value will always take precedence.

Consider whether this is the intended behavior. If initialMarkdown should only be used when localStorage is empty, this is correct. Otherwise, you may want to add logic to handle updates to the initialMarkdown prop, or document this behavior clearly.

Suggested change

// Update markdownText if initialMarkdown prop changes and localStorage is empty

useEffect(() => {

const saved = localStorage.getItem('airdeck-markdown');

if (!saved && isLoaded) {

setMarkdownText(initialMarkdown);

}

}, [initialMarkdown, isLoaded]);

dannycahyo · 2025-12-09T15:19:43Z

Hi @ndrvndr , thank you so much for your initiative to improve this project. I highly appreciate your effort!

Before I review further, I think you can consider the feedback from Copilot.

ndrvndr · 2025-12-11T00:27:26Z

Hi @ndrvndr , thank you so much for your initiative to improve this project. I highly appreciate your effort!

Before I review further, I think you can consider the feedback from Copilot.

Hi @dannycahyo, thank you very much for your response. I will review the feedback from Copilot again during my work breaks. Thank you as well for your patience and understanding.

…s-platform - Use lazy initialization to prevent SSR hydration errors - Add STORAGE_KEY constant for better maintainability - Add isLoaded flag to prevent race conditions on mount - Add try-catch for localStorage error handling - Fix regex to support Windows (\r\n) and Unix (\n) line endings - Add auto-reset for currentSlide when out of bounds Fixes hydration errors when using SSR (ssr: true in react-router.config.ts)

ndrvndr · 2025-12-19T13:32:03Z

Hi @dannycahyo, i've addressed the SSR hydration issue you mentioned in the code review.

Changes made:
✅ Moved localStorage access to lazy initialization
✅ Added proper SSR checks with typeof window
✅ Implemented isLoaded flag to prevent race conditions
✅ Added error handling and cross-platform support
✅ Fixed potential out-of-bounds bug with currentSlide

The latest commit is now pushed to the PR. Let me know if there's anything else that needs adjustment!

dannycahyo · 2025-12-19T14:46:49Z

app/contexts/PresentationContext.tsx

+  useEffect(() => {
+    if (currentSlide >= totalSlides && totalSlides > 0) {
+      setCurrentSlide(totalSlides - 1);
+    }
+  }, [currentSlide, totalSlides]);


Nice initiative! I didn't realize the possibility of the out-of-bounds bug.

But this effect seems unnecessary for me, this official docummentation can be a good companion for you related why we might need an effect:

https://react.dev/learn/escape-hatches#you-might-not-need-an-effect

Instead of handling the out-of-bounds slide with effect, what about disabling the next button?

We can still use this logic:

if (currentSlide >= totalSlides && totalSlides > 0) {}

dannycahyo · 2025-12-19T14:51:45Z

app/contexts/PresentationContext.tsx

+  const [markdownText, setMarkdownText] = useState(() => {
+    if (typeof window !== 'undefined') {
+      return localStorage.getItem(STORAGE_KEY) || initialMarkdown;
+    }
+    return initialMarkdown;
+  });


We might need to use the local storage for other cases, that's why could you help me create custom hooks for handling local storage? It would be very helpful ^^

To give you guidance about the custom hooks for it, you may see this article as a reference:
https://www.geeksforgeeks.org/reactjs/reactjs-uselocalstorage-custom-hook/

dannycahyo

Please kindly consider my feedback ^^

…redundant useEffect - Replace manual slide index synchronization with derived safe index logic. - Remove redundant useEffect for out-of-bounds slide handling. - Wrap Context value in useMemo to prevent unnecessary consumer re-renders. - Add canNext and canPrev helper booleans for UI state management.

Add a reusable React custom hook for persisting state in localStorage. This hook: - Supports TypeScript generics - Reads initial value from localStorage on mount - Persists state changes to localStorage - Supports functional updates like useState - Safely handles JSON parsing and storage errors Purpose: - Reduce duplicated localStorage logic - Simplify persistent state management in React applications

…h useLocalStorage hook Refactor PresentationProvider to use the shared useLocalStorage hook for managing markdown persistence. Changes: - Remove manual localStorage initialization and side effects - Replace markdownText state with useLocalStorage - Simplify component logic and reduce boilerplate - Improve consistency and reusability of persistence logic No behavior change intended.

ndrvndr · 2025-12-31T12:22:53Z

Please kindly consider my feedback ^^

Hi @dannycahyo ,
I’ve pushed several commits addressing the changes suggested in the previous code review.

Apologies for the delay, and thank you very much for your patience and for taking the time to review my code 🙏

Please let me know if there’s anything else that needs adjustment or improvement.

dannycahyo requested a review from Copilot December 9, 2025 15:11

Copilot started reviewing on behalf of dannycahyo December 9, 2025 15:11 View session

Copilot AI reviewed Dec 9, 2025

View reviewed changes

dannycahyo reviewed Dec 19, 2025

View reviewed changes

dannycahyo requested changes Dec 19, 2025

View reviewed changes

ndrvndr added 3 commits December 31, 2025 18:54

+  // Update markdownText if initialMarkdown prop changes and localStorage is empty
+  useEffect(() => {
+    const saved = localStorage.getItem('airdeck-markdown');
+    if (!saved && isLoaded) {
+      setMarkdownText(initialMarkdown);
+    }
+  }, [initialMarkdown, isLoaded]);

Comments

Conversation

ndrvndr commented Dec 5, 2025

Uh oh!

ndrvndr commented Dec 6, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

dannycahyo commented Dec 9, 2025

Uh oh!

ndrvndr commented Dec 11, 2025

Uh oh!

ndrvndr commented Dec 19, 2025

Uh oh!

dannycahyo Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

dannycahyo Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

dannycahyo Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

dannycahyo left a comment

Choose a reason for hiding this comment

Uh oh!

ndrvndr commented Dec 31, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants