feat(gateway): redesign frontend with premium SaaS aesthetic - #6
Conversation
- Add glassmorphism effects with backdrop blur - Implement smooth animations and transitions - Add logo display and favicon - Show delete key after URL shortening - Date picker for expiration with seconds conversion - Copy buttons with Copied feedback - Proper loading states for buttons
Summary of ChangesHello, 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 overhauls the user interface of the URL shortener, transforming its appearance and enhancing usability. The changes aim to provide a more modern, intuitive, and visually appealing experience for users, making common actions like shortening and deleting URLs more streamlined and interactive. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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. 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 major and impressive redesign of the frontend, giving it a modern 'premium SaaS' aesthetic. The changes include a complete overhaul of the CSS, semantic improvements to the HTML structure, and new JavaScript functionality for better user feedback, such as loading states and copy animations. My review focuses on improving consistency and robustness in the new JavaScript helpers.
| function setBtnLoading(btnId, isLoading, text) { | ||
| const btn = document.getElementById(btnId); | ||
| const span = btn.querySelector('span'); | ||
| if (isLoading) { | ||
| btn.disabled = true; | ||
| btn.innerHTML = '<div class="spinner"></div><span>' + text + '</span>'; | ||
| } else { | ||
| btn.disabled = false; | ||
| btn.innerHTML = '<span>' + text + '</span>'; | ||
| } | ||
| } |
There was a problem hiding this comment.
Using innerHTML can introduce security vulnerabilities (XSS) if the text parameter contains untrusted content. Although it's safe in the current context with hardcoded strings, it's best practice to avoid innerHTML for setting text. Using DOM manipulation methods like replaceChildren and textContent is more secure and robust. This change also removes an unused span variable.
| function setBtnLoading(btnId, isLoading, text) { | |
| const btn = document.getElementById(btnId); | |
| const span = btn.querySelector('span'); | |
| if (isLoading) { | |
| btn.disabled = true; | |
| btn.innerHTML = '<div class="spinner"></div><span>' + text + '</span>'; | |
| } else { | |
| btn.disabled = false; | |
| btn.innerHTML = '<span>' + text + '</span>'; | |
| } | |
| } | |
| function setBtnLoading(btnId, isLoading, text) { | |
| const btn = document.getElementById(btnId); | |
| btn.disabled = isLoading; | |
| const textSpan = document.createElement('span'); | |
| textSpan.textContent = text; | |
| if (isLoading) { | |
| const spinner = document.createElement('div'); | |
| spinner.className = 'spinner'; | |
| btn.replaceChildren(spinner, textSpan); | |
| } else { | |
| btn.replaceChildren(textSpan); | |
| } | |
| } |
| .btn-danger { | ||
| width: 100%; | ||
| background: transparent; | ||
| color: var(--danger); | ||
| border: 1px solid var(--danger); | ||
| } | ||
| .btn-danger:hover { background: rgba(244, 63, 94, 0.15); } | ||
| .result { | ||
| margin-top: 1rem; | ||
| font-family: inherit; | ||
| font-weight: 600; | ||
| font-size: 1rem; | ||
| padding: 1rem; | ||
| background: var(--bg); | ||
| border-radius: 12px; | ||
| border: 1px solid rgba(255, 69, 58, 0.2); | ||
| cursor: pointer; | ||
| transition: var(--transition); | ||
| } |
There was a problem hiding this comment.
To support a loading indicator with a spinner and text (similar to .btn-primary), the .btn-danger class should also use flexbox for layout. This will ensure consistent behavior for loading states across different button types and improve maintainability.
| .btn-danger { | |
| width: 100%; | |
| background: transparent; | |
| color: var(--danger); | |
| border: 1px solid var(--danger); | |
| } | |
| .btn-danger:hover { background: rgba(244, 63, 94, 0.15); } | |
| .result { | |
| margin-top: 1rem; | |
| font-family: inherit; | |
| font-weight: 600; | |
| font-size: 1rem; | |
| padding: 1rem; | |
| background: var(--bg); | |
| border-radius: 12px; | |
| border: 1px solid rgba(255, 69, 58, 0.2); | |
| cursor: pointer; | |
| transition: var(--transition); | |
| } | |
| .btn-danger { | |
| width: 100%; | |
| background: transparent; | |
| color: var(--danger); | |
| font-family: inherit; | |
| font-weight: 600; | |
| font-size: 1rem; | |
| padding: 1rem; | |
| border-radius: 12px; | |
| border: 1px solid rgba(255, 69, 58, 0.2); | |
| cursor: pointer; | |
| transition: var(--transition); | |
| display: flex; | |
| align-items: center; | |
| justify-content: center; | |
| gap: 0.5rem; | |
| } |
| const btn = document.getElementById('delete-btn'); | ||
| const origText = btn.textContent; | ||
| btn.disabled = true; | ||
| btn.textContent = 'Deleting...'; | ||
|
|
||
| try { | ||
| const res = await fetch(base + '/r/' + encodeURIComponent(path) + '?delete_key=' + encodeURIComponent(key), { method: 'DELETE' }); | ||
| const data = await res.json(); | ||
| if (!res.ok) { | ||
| showMessage('delete-message', data.detail || 'Delete failed', 'error'); | ||
| return; | ||
| } | ||
| showMessage('delete-message', 'Deleted: ' + path + ' (cache purged via Redpanda)', 'success'); | ||
| showMessage('delete-message', 'Link deleted successfully.', 'success'); | ||
| document.getElementById('delete-path').value = ''; | ||
| document.getElementById('delete-key').value = ''; | ||
| lastShortPath = ''; | ||
| lastDeleteKey = ''; | ||
| } catch (e) { | ||
| showMessage('delete-message', 'Network error: ' + e.message, 'error'); | ||
| } finally { | ||
| btn.disabled = false; | ||
| btn.textContent = origText; | ||
| } |
There was a problem hiding this comment.
The loading state for the delete button is implemented manually. For consistency and better maintainability, you should reuse the setBtnLoading helper function, which is already used for the 'Shorten' button.
| const btn = document.getElementById('delete-btn'); | |
| const origText = btn.textContent; | |
| btn.disabled = true; | |
| btn.textContent = 'Deleting...'; | |
| try { | |
| const res = await fetch(base + '/r/' + encodeURIComponent(path) + '?delete_key=' + encodeURIComponent(key), { method: 'DELETE' }); | |
| const data = await res.json(); | |
| if (!res.ok) { | |
| showMessage('delete-message', data.detail || 'Delete failed', 'error'); | |
| return; | |
| } | |
| showMessage('delete-message', 'Deleted: ' + path + ' (cache purged via Redpanda)', 'success'); | |
| showMessage('delete-message', 'Link deleted successfully.', 'success'); | |
| document.getElementById('delete-path').value = ''; | |
| document.getElementById('delete-key').value = ''; | |
| lastShortPath = ''; | |
| lastDeleteKey = ''; | |
| } catch (e) { | |
| showMessage('delete-message', 'Network error: ' + e.message, 'error'); | |
| } finally { | |
| btn.disabled = false; | |
| btn.textContent = origText; | |
| } | |
| setBtnLoading('delete-btn', true, 'Deleting...'); | |
| try { | |
| const res = await fetch(base + '/r/' + encodeURIComponent(path) + '?delete_key=' + encodeURIComponent(key), { method: 'DELETE' }); | |
| const data = await res.json(); | |
| if (!res.ok) { | |
| showMessage('delete-message', data.detail || 'Delete failed', 'error'); | |
| return; | |
| } | |
| showMessage('delete-message', 'Link deleted successfully.', 'success'); | |
| document.getElementById('delete-path').value = ''; | |
| document.getElementById('delete-key').value = ''; | |
| lastShortPath = ''; | |
| lastDeleteKey = ''; | |
| } catch (e) { | |
| showMessage('delete-message', 'Network error: ' + e.message, 'error'); | |
| } finally { | |
| setBtnLoading('delete-btn', false, 'Delete Link'); | |
| } |
- Replace innerHTML with DOM manipulation in setBtnLoading (XSS prevention) - Add flexbox to .btn-danger for spinner + text alignment - Use setBtnLoading helper in delete button handler
|
All review comments addressed:
Changes pushed to branch. |
1 participant
Summary
Changes
gateway/static/index.html- Complete UI redesignTesting