Merge pull request #211 from fix/coderabbit-review-comments

ayoub3bidi · web-flow · commit 9fd279c8a8f1 · 2026-07-25T16:22:45.000+01:00
fix: address all CodeRabbit review comments from PR #210
diff --git a/src/components/ArrayVisualizer.jsx b/src/components/ArrayVisualizer.jsx
@@ -183,9 +183,26 @@ function ArrayVisualizer({
               <motion.div
                 initial={{ opacity: 0 }}
                 animate={{ opacity: 1 }}
-                className="absolute inset-0 bg-black/30 z-10"
-                aria-hidden="true"
-              />
+                className="absolute inset-0 z-10 flex items-center justify-center bg-black/80"
+                role="button"
+                tabIndex={0}
+                onClick={() => onGatedFeatureClick?.('complexity_limit')}
+                onKeyDown={e => {
+                  if (e.key === 'Enter' || e.key === ' ') {
+                    e.preventDefault();
+                    onGatedFeatureClick?.('complexity_limit');
+                  }
+                }}
+              >
+                <div className="text-center text-white p-6 max-w-xs">
+                  <p className="text-lg font-semibold mb-2">
+                    {t('featureGate.complexity_limit')}
+                  </p>
+                  <p className="text-sm opacity-80">
+                    {t('featureGate.complexity_limit_description')}
+                  </p>
+                </div>
+              </motion.div>
             )}
           </div>
         ) : (
diff --git a/src/components/ErrorBoundary.jsx b/src/components/ErrorBoundary.jsx
@@ -4,13 +4,8 @@
  * See LICENSE for details.
  */
 
-import { Component } from 'react';
+import { Component, createElement } from 'react';
 
-/**
- * React error boundary that catches render errors in its subtree
- * and displays a non-destructive fallback instead of crashing the
- * entire application.
- */
 class ErrorBoundary extends Component {
   constructor(props) {
     super(props);
@@ -25,9 +20,39 @@ class ErrorBoundary extends Component {
     console.error('[ErrorBoundary]', error, info?.componentStack);
   }
 
+  componentDidUpdate(prevProps) {
+    if (prevProps.resetKey !== this.props.resetKey && this.state.hasError) {
+      this.setState({ hasError: false });
+    }
+  }
+
   render() {
     if (this.state.hasError) {
-      return this.props.fallback ?? null;
+      return (
+        this.props.fallback ??
+        createElement(
+          'div',
+          {
+            className:
+              'flex flex-col items-center justify-center h-full p-8 text-center',
+          },
+          createElement(
+            'p',
+            { className: 'text-lg font-semibold mb-2' },
+            'Something went wrong.'
+          ),
+          createElement(
+            'button',
+            {
+              type: 'button',
+              className:
+                'mt-2 px-4 py-2 rounded-lg bg-blue-600 text-white hover:bg-blue-700',
+              onClick: () => this.setState({ hasError: false }),
+            },
+            'Try again'
+          )
+        )
+      );
     }
     return this.props.children;
   }
diff --git a/src/components/ErrorBoundary.test.jsx b/src/components/ErrorBoundary.test.jsx
@@ -33,14 +33,35 @@ describe('ErrorBoundary', () => {
     spy.mockRestore();
   });
 
-  it('renders nothing when a child throws and no fallback provided', () => {
+  it('renders built-in fallback when a child throws and no fallback provided', () => {
     const spy = vi.spyOn(console, 'error').mockImplementation(() => {});
-    const { container } = render(
+    render(
       <ErrorBoundary>
         <Boom />
       </ErrorBoundary>
     );
-    expect(container.innerHTML).toBe('');
+    expect(screen.getByText('Something went wrong.')).toBeInTheDocument();
+    expect(
+      screen.getByRole('button', { name: 'Try again' })
+    ).toBeInTheDocument();
+    spy.mockRestore();
+  });
+
+  it('recovers when resetKey changes', () => {
+    const spy = vi.spyOn(console, 'error').mockImplementation(() => {});
+    const { rerender } = render(
+      <ErrorBoundary resetKey={1}>
+        <Boom />
+      </ErrorBoundary>
+    );
+    expect(screen.getByText('Something went wrong.')).toBeInTheDocument();
+
+    rerender(
+      <ErrorBoundary resetKey={2}>
+        <span>recovered content</span>
+      </ErrorBoundary>
+    );
+    expect(screen.getByText('recovered content')).toBeInTheDocument();
     spy.mockRestore();
   });
 });
diff --git a/src/components/Footer.jsx b/src/components/Footer.jsx
@@ -181,7 +181,7 @@ function Footer() {
             </p>
             <div className="flex items-center gap-2">
               <a
-                href="https://github.com/ayoub3bidi/bayan-flow/blob/develop/LICENSE"
+                href={`${GITHUB_REPO_URL}/blob/main/LICENSE`}
                 target="_blank"
                 rel="noopener noreferrer"
                 className="inline-flex items-center px-2.5 py-0.5 rounded-full text-xs font-medium bg-orange-100 text-orange-800 border border-orange-200 hover:bg-orange-200 transition-colors"
diff --git a/src/components/GraphAlgorithmMatrixVisualizer.jsx b/src/components/GraphAlgorithmMatrixVisualizer.jsx
@@ -103,9 +103,26 @@ function GraphAlgorithmMatrixVisualizer({
           <motion.div
             initial={{ opacity: 0 }}
             animate={{ opacity: 1 }}
-            className="absolute inset-0 bg-black/30 z-10"
-            aria-hidden="true"
-          />
+            className="absolute inset-0 z-10 flex items-center justify-center bg-black/80"
+            role="button"
+            tabIndex={0}
+            onClick={() => onGatedFeatureClick?.('complexity_limit')}
+            onKeyDown={e => {
+              if (e.key === 'Enter' || e.key === ' ') {
+                e.preventDefault();
+                onGatedFeatureClick?.('complexity_limit');
+              }
+            }}
+          >
+            <div className="text-center text-white p-6 max-w-xs">
+              <p className="text-lg font-semibold mb-2">
+                {t('featureGate.complexity_limit')}
+              </p>
+              <p className="text-sm opacity-80">
+                {t('featureGate.complexity_limit_description')}
+              </p>
+            </div>
+          </motion.div>
         )}
       </div>
     );
diff --git a/src/components/GraphAlgorithmMatrixVisualizer.test.jsx b/src/components/GraphAlgorithmMatrixVisualizer.test.jsx
@@ -120,9 +120,11 @@ describe('GraphAlgorithmMatrixVisualizer', () => {
         vi.advanceTimersByTime(1000);
       });
 
-      const blurOverlay = document.querySelector('.bg-black\\/30');
+      const blurOverlay = document.querySelector('.bg-black\\/80');
       expect(blurOverlay).toBeInTheDocument();
-      expect(screen.getByText('Complexity Analysis')).toBeInTheDocument();
+      expect(
+        screen.getAllByText('Complexity Analysis').length
+      ).toBeGreaterThanOrEqual(1);
     } finally {
       vi.useRealTimers();
     }
diff --git a/src/components/GraphVisualizer.jsx b/src/components/GraphVisualizer.jsx
@@ -321,9 +321,26 @@ function GraphVisualizer({
               <motion.div
                 initial={{ opacity: 0 }}
                 animate={{ opacity: 1 }}
-                className="absolute inset-0 bg-black/30 z-10"
-                aria-hidden="true"
-              />
+                className="absolute inset-0 z-10 flex items-center justify-center bg-black/80"
+                role="button"
+                tabIndex={0}
+                onClick={() => onGatedFeatureClick?.('complexity_limit')}
+                onKeyDown={e => {
+                  if (e.key === 'Enter' || e.key === ' ') {
+                    e.preventDefault();
+                    onGatedFeatureClick?.('complexity_limit');
+                  }
+                }}
+              >
+                <div className="text-center text-white p-6 max-w-xs">
+                  <p className="text-lg font-semibold mb-2">
+                    {t('featureGate.complexity_limit')}
+                  </p>
+                  <p className="text-sm opacity-80">
+                    {t('featureGate.complexity_limit_description')}
+                  </p>
+                </div>
+              </motion.div>
             )}
           </div>
         ) : (
diff --git a/src/components/GraphVisualizer.test.jsx b/src/components/GraphVisualizer.test.jsx
@@ -195,9 +195,11 @@ describe('GraphVisualizer', () => {
         vi.advanceTimersByTime(1000);
       });
 
-      const blurOverlay = document.querySelector('.bg-black\\/30');
+      const blurOverlay = document.querySelector('.bg-black\\/80');
       expect(blurOverlay).toBeInTheDocument();
-      expect(screen.getByText('Complexity Analysis')).toBeInTheDocument();
+      expect(
+        screen.getAllByText('Complexity Analysis').length
+      ).toBeGreaterThanOrEqual(1);
     } finally {
       vi.useRealTimers();
     }
diff --git a/src/components/GridVisualizer.jsx b/src/components/GridVisualizer.jsx
@@ -182,9 +182,26 @@ function GridVisualizer({
               <motion.div
                 initial={{ opacity: 0 }}
                 animate={{ opacity: 1 }}
-                className="absolute inset-0 bg-black/30 z-10"
-                aria-hidden="true"
-              />
+                className="absolute inset-0 z-10 flex items-center justify-center bg-black/80"
+                role="button"
+                tabIndex={0}
+                onClick={() => onGatedFeatureClick?.('complexity_limit')}
+                onKeyDown={e => {
+                  if (e.key === 'Enter' || e.key === ' ') {
+                    e.preventDefault();
+                    onGatedFeatureClick?.('complexity_limit');
+                  }
+                }}
+              >
+                <div className="text-center text-white p-6 max-w-xs">
+                  <p className="text-lg font-semibold mb-2">
+                    {t('featureGate.complexity_limit')}
+                  </p>
+                  <p className="text-sm opacity-80">
+                    {t('featureGate.complexity_limit_description')}
+                  </p>
+                </div>
+              </motion.div>
             )}
           </div>
         ) : (
diff --git a/src/components/PythonCodePanel.jsx b/src/components/PythonCodePanel.jsx
@@ -15,6 +15,7 @@ import {
 } from '../algorithms/python';
 import { getPseudocodeForLocale } from '../algorithms/pseudocode';
 import { highlightPseudocodeToHtml } from '../utils/pseudocodeHighlight';
+import { getFocusableElements } from '../utils/focusableElements';
 import Editor from '@monaco-editor/react';
 import { useTheme } from '../hooks/useTheme';
 import { usePythonExecution } from '../hooks/usePythonExecution';
@@ -78,20 +79,17 @@ function PythonCodePanel({ isOpen, onClose, algorithm }) {
     document.addEventListener('keydown', handleEscape);
     return () => document.removeEventListener('keydown', handleEscape);
   }, [isOpen, onClose]);
-
   // Focus trap
   useEffect(() => {
     if (isOpen && panelRef.current) {
-      const focusableElements = panelRef.current.querySelectorAll(
-        'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])'
-      );
-      const firstElement = focusableElements[0];
-      const lastElement = focusableElements[focusableElements.length - 1];
-
       const handleKeyDown = event => {
         if (event.key === 'Escape') {
           onClose();
         } else if (event.key === 'Tab') {
+          const focusableElements = getFocusableElements(panelRef.current);
+          const firstElement = focusableElements[0];
+          const lastElement = focusableElements[focusableElements.length - 1];
+
           if (event.shiftKey) {
             if (document.activeElement === firstElement) {
               event.preventDefault();
@@ -107,12 +105,11 @@ function PythonCodePanel({ isOpen, onClose, algorithm }) {
       };
 
       document.addEventListener('keydown', handleKeyDown);
-      firstElement?.focus();
+      getFocusableElements(panelRef.current)[0]?.focus();
 
       return () => document.removeEventListener('keydown', handleKeyDown);
     }
   }, [isOpen, onClose]);
-
   const pythonCode = getPythonCode(algorithm);
   const displayName = getAlgorithmDisplayName(algorithm);
   const pseudocodeText = useMemo(
diff --git a/src/components/SettingsSheet.jsx b/src/components/SettingsSheet.jsx
@@ -15,6 +15,7 @@ import {
   sheetPanelVariants,
 } from '../motion/chromeMotion';
 import { useBodyScrollLock } from '../hooks/useBodyScrollLock';
+import { getFocusableElements } from '../utils/focusableElements';
 import SettingsPanel from './SettingsPanel';
 
 /**
@@ -26,13 +27,28 @@ function SettingsSheet({ isOpen, onClose, settingsPanelProps }) {
   const panelRef = useRef(null);
 
   useBodyScrollLock(isOpen);
-
   useEffect(() => {
     if (!isOpen) return undefined;
 
     const handleKeyDown = event => {
       if (event.key === 'Escape') {
         onClose();
+      } else if (event.key === 'Tab' && panelRef.current) {
+        const focusableElements = getFocusableElements(panelRef.current);
+        const firstElement = focusableElements[0];
+        const lastElement = focusableElements[focusableElements.length - 1];
+
+        if (event.shiftKey) {
+          if (document.activeElement === firstElement) {
+            event.preventDefault();
+            lastElement.focus();
+          }
+        } else {
+          if (document.activeElement === lastElement) {
+            event.preventDefault();
+            firstElement.focus();
+          }
+        }
       }
     };
 
diff --git a/src/components/ShareExportModal.jsx b/src/components/ShareExportModal.jsx
@@ -4,7 +4,7 @@
  * See LICENSE for details.
  */
 
-import { useState } from 'react';
+import { useState, useEffect } from 'react';
 import { useTranslation } from 'react-i18next';
 import { Share, CopySimple, Check, X } from '@phosphor-icons/react';
 import { SiX } from 'react-icons/si';
@@ -39,10 +39,17 @@ function ShareExportModal({
 }) {
   const { t } = useTranslation();
   const reduceMotion = useReducedMotion();
-  const shareData = generateShareCaption(algorithmName);
+  const shareData = generateShareCaption(algorithmName, t);
   const [caption, setCaption] = useState(shareData.fullShareText);
   const [copied, setCopied] = useState(false);
 
+  useEffect(() => {
+    if (open) {
+      setCaption(generateShareCaption(algorithmName, t).fullShareText);
+      setCopied(false);
+    }
+  }, [open, algorithmName, t]);
+
   const handleNativeShare = async () => {
     if (!videoBlob) return;
     const file = new File([videoBlob], videoFileName, { type: 'video/mp4' });
diff --git a/src/components/ShareExportModal.test.jsx b/src/components/ShareExportModal.test.jsx
@@ -17,11 +17,25 @@ const mockTranslations = {
   'shareExport.copyCaption': 'Copy caption',
   'shareExport.copied': 'Copied!',
   'shareExport.close': 'Close',
+  'shareExport.captionTemplate':
+    '{{algorithm}}. Step-by-step visualization from Bayan Flow.',
+  'shareExport.titleTemplate': '{{algorithm}} Visualization — Bayan Flow',
+};
+
+const stableT = (key, opts) => {
+  const val = mockTranslations[key] || key;
+  if (opts && typeof val === 'string') {
+    return Object.entries(opts).reduce(
+      (s, [k, v]) => s.replace(new RegExp(`\\{\\{${k}\\}\\}`, 'g'), v),
+      val
+    );
+  }
+  return val;
 };
 
 vi.mock('react-i18next', () => ({
   useTranslation: () => ({
-    t: key => mockTranslations[key] || key,
+    t: stableT,
     i18n: { language: 'en' },
   }),
 }));
diff --git a/src/components/TreeVisualizer.jsx b/src/components/TreeVisualizer.jsx
diff --git a/src/i18n/locales/ar/translation.json b/src/i18n/locales/ar/translation.json
diff --git a/src/i18n/locales/en/translation.json b/src/i18n/locales/en/translation.json
diff --git a/src/i18n/locales/fr/translation.json b/src/i18n/locales/fr/translation.json
diff --git a/src/utils/focusableElements.js b/src/utils/focusableElements.js
diff --git a/src/utils/shareCaption.js b/src/utils/shareCaption.js
