fix(frontend): streamline web ux (#916)

## Summary
- streamline the conversion flow UI and strategy handling
- normalize user-entered URLs before submit and auto-submit
- improve access-token storage fallback cleanup for restricted session
storage contexts

## Validation
- make ready
- frontend smoke at http://127.0.0.1:4001/ (initial + token-required
states; action uniqueness checked)

Author: gildesmarais

app/web/api/v1/create_feed.rb

@@ -61,16 +61,46 @@ def build_create_params(params, account)
             # @param account [Hash]
             # @return [String]
             def validated_url(raw_url, account)
-              url = raw_url.to_s.strip
+              url = normalized_input_url(raw_url)
               raise Html2rss::Web::BadRequestError, 'URL parameter is required' if url.empty?
-              raise Html2rss::Web::BadRequestError, 'Invalid URL format' unless UrlValidator.valid_url?(url)
+
+              url = UrlValidator.canonical_url(url)
+              raise Html2rss::Web::BadRequestError, 'Invalid URL format' unless url
               unless UrlValidator.url_allowed?(account, url)
                 raise Html2rss::Web::ForbiddenError, 'URL not allowed for this account'
               end
 
               url
             end
 
+            # @param raw_url [String, nil]
+            # @return [String]
+            def normalized_input_url(raw_url)
+              url = raw_url.to_s.strip
+              return url if url.empty?
+              return "https:#{url}" if url.start_with?('//')
+              return url if absolute_url?(url)
+
+              hostname_input?(url) ? "https://#{url}" : url
+            end
+
+            # @param url [String]
+            # @return [Boolean]
+            def absolute_url?(url)
+              url.match?(%r{\A[a-z][a-z0-9+\-.]*://}i)
+            end
+
+            # @param url [String]
+            # @return [Boolean]
+            def hostname_input?(url)
+              %r{
+                \A
+                (localhost(?::\d+)?|(?:\d{1,3}\.){3}\d{1,3}(?::\d+)?|(?:[a-z0-9-]+\.)+[a-z]{2,}(?::\d+)?)
+                (?:[/?#].*)?
+                \z
+              }ix.match?(url)
+            end
+
             # @param raw_strategy [String, nil]
             # @return [String]
             def normalize_strategy(raw_strategy)

app/web/security/url_validator.rb

@@ -10,10 +10,16 @@ module UrlValidator
       MAX_URL_LENGTH = 2048
 
       class << self
+        # @param url [String]
+        # @return [String, nil]
+        def canonical_url(url)
+          normalize_url(url)
+        end
+
         # @param url [String]
         # @return [Boolean]
         def valid_url?(url)
-          !normalize_url(url).nil?
+          !canonical_url(url).nil?
         end
 
         # @param account [Hash]
@@ -23,7 +29,7 @@ def url_allowed?(account, url)
           return false unless account && url
 
           allowed_urls = Array(account[:allowed_urls])
-          return false unless (normalized_url = normalize_url(url))
+          return false unless (normalized_url = canonical_url(url))
 
           return false if allowed_urls.empty?
 
@@ -37,7 +43,7 @@ def url_allowed?(account, url)
         def match_exact?(pattern, normalized_url)
           return true if pattern == normalized_url
 
-          normalized_pattern = normalize_url(pattern)
+          normalized_pattern = canonical_url(pattern)
           normalized_pattern ? normalized_pattern == normalized_url : false
         end
 

frontend/src/__tests__/App.contract.test.tsx

@@ -8,7 +8,7 @@ describe('App contract', () => {
   const token = 'contract-token';
 
   const authenticate = () => {
-    window.sessionStorage.setItem('html2rss_access_token', token);
+    window.localStorage.setItem('html2rss_access_token', token);
   };
 
   it('shows feed result when API responds with success', async () => {
@@ -18,7 +18,7 @@ describe('App contract', () => {
       http.post('/api/v1/feeds', async ({ request }) => {
         const body = (await request.json()) as { url: string; strategy: string };
 
-        expect(body).toEqual({ url: 'https://example.com/articles', strategy: 'browserless' });
+        expect(body).toEqual({ url: 'https://example.com/articles', strategy: 'faraday' });
         expect(request.headers.get('authorization')).toBe(`Bearer ${token}`);
 
         return HttpResponse.json(
@@ -55,7 +55,7 @@ describe('App contract', () => {
 
     await screen.findByLabelText('Page URL');
     await waitFor(() => {
-      expect(screen.getByRole('combobox')).toHaveValue('browserless');
+      expect(screen.getByRole('combobox')).toHaveValue('faraday');
     });
 
     const urlInput = screen.getByLabelText('Page URL') as HTMLInputElement;
@@ -149,7 +149,7 @@ describe('App contract', () => {
 
     await screen.findByLabelText('Page URL');
     await waitFor(() => {
-      expect(screen.getByRole('combobox')).toHaveValue('browserless');
+      expect(screen.getByRole('combobox')).toHaveValue('faraday');
     });
 
     fireEvent.input(screen.getByLabelText('Page URL'), {
@@ -161,6 +161,6 @@ describe('App contract', () => {
 
     expect(screen.getByText('Add access token')).toBeInTheDocument();
     expect(screen.queryByText('Feed generation failed')).not.toBeInTheDocument();
-    expect(window.sessionStorage.getItem('html2rss_access_token')).toBeNull();
+    expect(window.localStorage.getItem('html2rss_access_token')).toBeNull();
   });
 });

frontend/src/__tests__/App.test.tsx

@@ -91,11 +91,22 @@ describe('App', () => {
     render(<App />);
 
     expect(screen.getByLabelText('html2rss')).toBeInTheDocument();
+    expect(screen.getByRole('link', { name: 'html2rss' })).toHaveAttribute('href', '/');
     expect(screen.getByLabelText('Page URL')).toBeInTheDocument();
     expect(screen.getByRole('button', { name: 'More' })).toBeInTheDocument();
     expect(screen.queryByRole('link', { name: 'Bookmarklet' })).not.toBeInTheDocument();
   });
 
+  it('keeps the page url field permissive enough for hostname-only input', () => {
+    render(<App />);
+
+    const urlInput = screen.getByLabelText('Page URL');
+
+    expect(urlInput).toHaveAttribute('type', 'text');
+    expect(urlInput).toHaveAttribute('inputmode', 'url');
+    expect(urlInput).toHaveAttribute('autocapitalize', 'off');
+  });
+
   it('autofocuses the source url field', async () => {
     render(<App />);
 
@@ -104,11 +115,11 @@ describe('App', () => {
     });
   });
 
-  it('prefers browserless as the default strategy when available', () => {
+  it('prefers faraday as the default strategy when available', () => {
     render(<App />);
 
     return waitFor(() => {
-      expect(screen.getByRole('combobox')).toHaveValue('browserless');
+      expect(screen.getByRole('combobox')).toHaveValue('faraday');
     });
   });
 
@@ -140,11 +151,7 @@ describe('App', () => {
     render(<App />);
 
     await waitFor(() => {
-      expect(mockConvertFeed).toHaveBeenCalledWith(
-        'https://example.com/articles',
-        'browserless',
-        'saved-token'
-      );
+      expect(mockConvertFeed).toHaveBeenCalledWith('https://example.com/articles', 'faraday', 'saved-token');
     });
   });
 
@@ -221,7 +228,9 @@ describe('App', () => {
         preview: {
           items: [],
           error: 'Preview unavailable right now.',
+          isLoading: false,
         },
+        retry: null,
       },
       error: null,
       convertFeed: mockConvertFeed,
@@ -243,6 +252,7 @@ describe('App', () => {
       result: null,
       error: 'Access denied',
       convertFeed: mockConvertFeed,
+      clearError: mockClearConversionError,
       clearResult: mockClearResult,
     });
 
@@ -331,11 +341,7 @@ describe('App', () => {
 
     await waitFor(() => {
       expect(mockSaveToken).toHaveBeenCalledWith('token-123');
-      expect(mockConvertFeed).toHaveBeenCalledWith(
-        'https://example.com/articles',
-        'browserless',
-        'token-123'
-      );
+      expect(mockConvertFeed).toHaveBeenCalledWith('https://example.com/articles', 'faraday', 'token-123');
     });
   });
 
@@ -419,13 +425,87 @@ describe('App', () => {
     expect(bookmarklet.getAttribute('href')).not.toContain('%27+encodeURIComponent');
   });
 
+  it('opens token entry immediately for bookmarklet urls when no token is saved', async () => {
+    window.history.replaceState({}, '', 'http://localhost:3000/?url=example.com%2Farticles');
+
+    render(<App />);
+
+    await screen.findByText('Add access token');
+    expect(screen.getByLabelText('Page URL')).toHaveValue('https://example.com/articles');
+    expect(mockConvertFeed).not.toHaveBeenCalled();
+  });
+
+  it('offers a direct alternate strategy retry after conversion failure', async () => {
+    mockUseAccessToken.mockReturnValue({
+      token: 'saved-token',
+      hasToken: true,
+      saveToken: mockSaveToken,
+      clearToken: mockClearToken,
+      isLoading: false,
+      error: null,
+    });
+    mockConvertFeed
+      .mockRejectedValueOnce(
+        Object.assign(new Error('Tried faraday first, then browserless. Browserless failed.'), {
+          manualRetryStrategy: 'browserless',
+        })
+      )
+      .mockResolvedValueOnce(undefined);
+
+    render(<App />);
+
+    fireEvent.input(screen.getByLabelText('Page URL'), {
+      target: { value: 'https://example.com/articles' },
+    });
+    fireEvent.click(screen.getByRole('button', { name: 'Generate feed URL' }));
+
+    await screen.findByRole('button', { name: 'Try browserless instead' });
+    fireEvent.click(screen.getByRole('button', { name: 'Try browserless instead' }));
+
+    await waitFor(() => {
+      expect(mockConvertFeed).toHaveBeenLastCalledWith(
+        'https://example.com/articles',
+        'browserless',
+        'saved-token'
+      );
+    });
+  });
+
+  it('does not offer a duplicate retry action after automatic fallback already failed', async () => {
+    mockUseAccessToken.mockReturnValue({
+      token: 'saved-token',
+      hasToken: true,
+      saveToken: mockSaveToken,
+      clearToken: mockClearToken,
+      isLoading: false,
+      error: null,
+    });
+    mockConvertFeed.mockRejectedValueOnce(
+      Object.assign(new Error('Tried faraday first, then browserless. Browserless failed.'), {
+        manualRetryStrategy: '',
+      })
+    );
+
+    render(<App />);
+
+    fireEvent.input(screen.getByLabelText('Page URL'), {
+      target: { value: 'https://example.com/articles' },
+    });
+    fireEvent.click(screen.getByRole('button', { name: 'Generate feed URL' }));
+
+    await screen.findByText('Tried faraday first, then browserless. Browserless failed.');
+    expect(screen.queryByRole('button', { name: /Try .* instead/ })).not.toBeInTheDocument();
+  });
+
   it('shows the utility links in a user-focused order', () => {
     window.history.replaceState({}, '', 'http://localhost:3000/#result');
     render(<App />);
 
     fireEvent.click(screen.getByRole('button', { name: 'More' }));
 
-    const utilityLinks = screen.getAllByRole('link').map((link) => link.textContent);
+    const utilityLinks = Array.from(
+      screen.getByLabelText('Utilities').querySelectorAll('.utility-strip__items > a')
+    ).map((link) => link.textContent);
     expect(utilityLinks).toEqual([
       'Try included feeds',
       'Bookmarklet',

frontend/src/__tests__/ResultDisplay.test.tsx

@@ -36,7 +36,9 @@ describe('ResultDisplay', () => {
         },
       ],
       error: null,
+      isLoading: false,
     },
+    retry: null,
   };
 
   beforeEach(() => {
@@ -49,6 +51,10 @@ describe('ResultDisplay', () => {
     expect(screen.getByText('Your feed is ready')).toBeInTheDocument();
     expect(screen.getByText('Test Feed')).toBeInTheDocument();
     expect(screen.getByRole('button', { name: 'Copy feed URL' })).toBeInTheDocument();
+    expect(screen.getByRole('link', { name: 'Subscribe in reader' })).toHaveAttribute(
+      'href',
+      'feed:https://example.com/feed.xml'
+    );
     expect(screen.getByRole('link', { name: 'Open feed' })).toBeInTheDocument();
     expect(screen.getByRole('link', { name: 'Open JSON Feed' })).toHaveAttribute(
       'href',
@@ -67,7 +73,10 @@ describe('ResultDisplay', () => {
   it('surfaces preview failures as a result-state message', async () => {
     render(
       <ResultDisplay
-        result={{ ...mockResult, preview: { items: [], error: 'Preview unavailable right now.' } }}
+        result={{
+          ...mockResult,
+          preview: { items: [], error: 'Preview unavailable right now.', isLoading: false },
+        }}
         onCreateAnother={mockOnCreateAnother}
       />
     );
@@ -78,6 +87,39 @@ describe('ResultDisplay', () => {
     });
   });
 
+  it('keeps the result state visible while preview is still loading', async () => {
+    render(
+      <ResultDisplay
+        result={{ ...mockResult, preview: { items: [], error: null, isLoading: true } }}
+        onCreateAnother={mockOnCreateAnother}
+      />
+    );
+
+    await waitFor(() => {
+      expect(screen.getByText('Your feed is ready')).toBeInTheDocument();
+      expect(screen.getByRole('link', { name: 'Open feed' })).toBeInTheDocument();
+      expect(screen.getByText('Loading preview…')).toBeInTheDocument();
+    });
+  });
+
+  it('shows an automatic retry notice when fallback strategy succeeded', async () => {
+    render(
+      <ResultDisplay
+        result={{
+          ...mockResult,
+          retry: { automatic: true, from: 'faraday', to: 'browserless' },
+        }}
+        onCreateAnother={mockOnCreateAnother}
+      />
+    );
+
+    await waitFor(() => {
+      expect(
+        screen.getByText('Retried automatically with browserless after faraday could not finish the page.')
+      ).toBeInTheDocument();
+    });
+  });
+
   it('calls onCreateAnother when the reset button is clicked', () => {
     render(<ResultDisplay result={mockResult} onCreateAnother={mockOnCreateAnother} />);