Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @Zingzy, your pull request is larger than the review limit of 150000 diff characters
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 43 minutes and 7 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (85)
📝 WalkthroughWalkthroughAdds a modular UI primitives layer, replaces the legacy notification system, introduces alias-availability API and client-side alias checker, centralizes analytics loading, introduces CSS design tokens and many CSS reorganizations, and consolidates/refactors dashboard scripts and templates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant AliasAPI
participant ShortenAPI
User->>Browser: Type alias in input
Browser->>Browser: AliasChecker.performClientValidation()
alt client validation passes
Browser->>AliasAPI: GET /api/v1/shorten/check-alias?alias=...
AliasAPI-->>Browser: { available: bool, reason: ... }
Browser->>Browser: update UI indicator via field-error/modal-tabs
else validation fails
Browser->>Browser: show immediate field error
end
User->>Browser: Submit create link form
Browser->>Browser: validate form, applyServerErrors if needed
alt form valid
Browser->>ShortenAPI: POST /api/v1/shorten (authFetch)
ShortenAPI-->>Browser: success response
Browser->>Browser: open success modal, render QR, copy to clipboard, trigger confetti, showNotification('success')
else server returns field errors
Browser->>Browser: applyServerErrors -> ModalTabs.jumpToFirstInvalid()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the frontend templates/assets by replacing the legacy customTopNotification system with a unified showNotification implementation, extracting repeated analytics markup into a reusable partial, and standardizing styling primitives (notably z-index tokens and status colors) across the site and dashboard.
Changes:
- Replaced
customNotificationJS/CSS usage in templates with the newstatic/js/notifications.js+static/css/notifications.css. - Introduced
templates/partials/analytics.htmland wired it into multiple base templates to deduplicate Sentry/Clarity injection. - Moved large inline dashboard Links page JS into
static/js/dashboard/links.jsand introduced additional dashboard settings assets.
Reviewed changes
Copilot reviewed 52 out of 56 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| templates/stats.html | Switches error notifications to showNotification. |
| templates/report.html | Switches success/error notifications to showNotification. |
| templates/password.html | Migrates to new notifications CSS/JS for password errors. |
| templates/partials/auth_modal.html | Removes large inline auth modal CSS from template. |
| templates/partials/analytics.html | New shared Sentry/Clarity injection partial. |
| templates/legal/base.html | Includes shared analytics + extracts contact modal into partial include. |
| templates/index.html | Uses showNotification for alias/URL errors and removes legacy includes. |
| templates/dashboard/statistics.html | Formatting-only tweaks and minor markup wrapping. |
| templates/dashboard/partials/verification_banner.html | CSS/JS indentation/formatting adjustments. |
| templates/dashboard/links.html | Removes inline Links page script and loads dashboard/links.js. |
| templates/dashboard/billing.html | Minor formatting-only changes. |
| templates/dashboard/base.html | Replaces legacy notification assets; extracts auth utilities to auth-utils.js; includes analytics partial. |
| templates/contact.html | Switches notifications to showNotification and removes legacy includes. |
| templates/base.html | Adds notifications/auth modal CSS and loads notifications.js; includes analytics partial. |
| templates/api.html | Includes shared analytics partial. |
| static/js/url-manager.js | Uses global showNotification instead of a class method. |
| static/js/notifications.js | New unified showNotification(message, type, duration) implementation. |
| static/js/index-validate.js | Uses showNotification for client-side validation errors. |
| static/js/index-script.js | Uses showNotification for API/network errors when shortening. |
| static/js/dashboard/settings.js | New Settings page JS (OAuth providers, profile pictures, set-password modal). |
| static/js/dashboard/links.js | New extracted Links page script (filters, modals, tooltips, create-link flow). |
| static/js/dashboard/keys.js | Replaces legacy notifications with showNotification. |
| static/js/dashboard.js | Removes old Links page script bundle (migrated into dashboard/links.js). |
| static/js/customNotification.js | Removes legacy custom notification implementation. |
| static/js/auth-utils.js | New shared dashboard auth helpers (authFetch, logout). |
| static/css/v2-announcement.css | Updates z-index usage and formatting. |
| static/css/stats-view.css | Minor formatting-only tweaks. |
| static/css/result.css | Minor formatting/cleanup (duplicate body rules removed). |
| static/css/report.css | Removes duplicate body rules; minor selector formatting. |
| static/css/prism-duotone-dark.css | Selector formatting normalization. |
| static/css/preview.css | Removes unused .preview-icon* styles. |
| static/css/password.css | Removes duplicate body blocks; simplifies background declarations. |
| static/css/notifications.css | New unified notification styling. |
| static/css/mobile-header.css | Uses shared z-index tokens; formatting normalization. |
| static/css/landing-edit-modal.css | Uses shared z-index token. |
| static/css/header.css | Uses shared z-index tokens for header/profile dropdown. |
| static/css/error.css | Formatting-only tweak. |
| static/css/docs.css | Formatting-only tweaks for selectors and media blocks. |
| static/css/dashboard/url-modal.css | Uses shared z-index tokens and shared status color variables. |
| static/css/dashboard/statistics.css | Uses shared tokens; removes some unused styles; moves AnyChart credits hiding. |
| static/css/dashboard/settings.css | New Settings page styling including modal styles. |
| static/css/dashboard/links.css | Uses shared tokens and status colors; adds success modal z-index. |
| static/css/dashboard/keys.css | Uses shared tokens and status colors. |
| static/css/dashboard/dateRangePicker.css | Uses shared tokens and status colors. |
| static/css/dashboard/dashboard-base.css | Uses shared z-index tokens for layout layering. |
| static/css/dashboard/billing.css | Formatting-only selector normalization. |
| static/css/dashboard.css | Removes legacy dashboard stylesheet (replaced by dashboard-specific CSS). |
| static/css/customNotification.css | Removes legacy custom notification CSS. |
| static/css/contacts-modal.css | Replaces extreme z-index with shared token; minor formatting. |
| static/css/contact.css | Removes duplicate body rules and updates selector formatting/IDs. |
| static/css/confetti.css | Formatting-only tweak. |
| static/css/base.css | Adds shared z-index scale and status color CSS variables. |
| static/css/auth.css | Formatting-only tweaks. |
| static/css/auth-modal.css | New extracted auth modal override stylesheet. |
| static/css/api.css | Formatting-only tweaks for selector grouping. |
Comments suppressed due to low confidence (1)
templates/base.html:52
notifications.jsis loaded withdefer, but several templates callshowNotification(...)via inline<script>tags in the body (e.g., when{% if error %}), which execute before deferred scripts run. This can throwReferenceError: showNotification is not definedand prevent page JS from running. Consider loadingnotifications.jsnon-deferred (or in<head>), or moving the inline notification triggers into{% block scripts %}/ a DOMContentLoaded handler.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
static/css/contact.css (1)
1-13:⚠️ Potential issue | 🟠 MajorAdd a solid fallback background color for readability.
With white foreground text, relying only on a data-URI background can render content unreadable when image loading is blocked/fails. Keep a
background-colorfallback beforebackground-image.Suggested fix
body { font-family: 'Allerta Stencil', 'Segoe UI', Tahoma, Geneva, Verdana, sans-serif; margin: 0; padding: 0; box-sizing: border-box; color: white; width: 100%; height: 100%; + background-color: `#040337`; background-size: cover; background-position: center center; background-repeat: repeat; background-image: url("data:image/svg+xml;utf8,..."); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/css/contact.css` around lines 1 - 13, The body rule currently only sets background-image (data URI) so if the SVG fails to load white text may be unreadable; add a solid dark fallback by inserting a background-color (e.g. background-color: `#040337`;) before the existing background-image declaration in the body selector to ensure readable contrast when the image is blocked or slow to load.static/css/dashboard/dateRangePicker.css (1)
220-236:⚠️ Potential issue | 🟡 MinorHover state has no visual feedback — same color as default state.
The
.apply-btn:hoverbackground (Line 235) uses the samevar(--accent-primary)as the default state (Line 222), removing any visual indication that the button is hovered. The original likely used a darker shade for hover feedback.Consider using a darker variant or adjusting the color for the hover state:
🎨 Suggested fix
.apply-btn:hover { - background: var(--accent-primary); + background: color-mix(in srgb, var(--accent-primary) 85%, black); }Or if a hover accent variable exists:
.apply-btn:hover { - background: var(--accent-primary); + background: var(--accent-primary-hover, `#6d28d9`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/css/dashboard/dateRangePicker.css` around lines 220 - 236, The hover state currently uses the same color as the default so there is no visual feedback; update the .apply-btn:hover rule to use a darker (or higher-contrast) color variant rather than var(--accent-primary) — for example switch to a darker token like var(--accent-primary-dark) or a slightly darkened hex/rgb value so hovering shows clear feedback while preserving existing .apply-btn styles and transitions.
🧹 Nitpick comments (13)
static/css/stats-view.css (1)
354-398: Consider simplifying the fade-in animation.The fade-in animation uses 11 keyframe steps to interpolate opacity from 0 to 1. CSS animations automatically interpolate between keyframes, so this can be simplified significantly without changing the visual effect.
♻️ Simplified animation
`@keyframes` fade-in { - 0% { + from { opacity: 0; } - - 10% { - opacity: 0.1; - } - - 20% { - opacity: 0.2; - } - - 30% { - opacity: 0.3; - } - - 40% { - opacity: 0.4; - } - - 50% { - opacity: 0.5; - } - - 60% { - opacity: 0.6; - } - - 70% { - opacity: 0.7; - } - - 80% { - opacity: 0.8; - } - - 90% { - opacity: 0.9; - } - - 100% { + to { opacity: 1; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/css/stats-view.css` around lines 354 - 398, The `@keyframes` fade-in definition is overly verbose; replace the multi-step opacity stops with a minimal equivalent (e.g., just 0% { opacity: 0; } and 100% { opacity: 1; }) so the browser will interpolate the in-between values automatically; update the `@keyframes` named fade-in accordingly to keep the same visual effect while simplifying the CSS.templates/dashboard/partials/verification_banner.html (1)
27-27: Consider using z-index CSS variable for consistency.The banner uses a hardcoded
z-index: 100, which matches--z-dropdownfrombase.css. Ifbase.cssis loaded before this partial, consider usingvar(--z-dropdown)for consistency with the new design system.♻️ Suggested change
- z-index: 100; + z-index: var(--z-dropdown);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/dashboard/partials/verification_banner.html` at line 27, Replace the hardcoded z-index value in the verification banner partial with the design-system CSS variable: locate the z-index declaration in verification_banner.html (the rule currently using "z-index: 100") and change it to use var(--z-dropdown) so the banner follows the shared --z-dropdown token from base.css.static/css/v2-announcement.css (1)
94-106: Optional: Consider kebab-case for keyframe names.Stylelint flags
borderShiftandemojiFloatas not following kebab-case convention (border-shift,emoji-float). This is a stylistic preference and not a functional issue, but aligning with kebab-case would be consistent with CSS naming conventions.Also applies to: 459-469
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/css/v2-announcement.css` around lines 94 - 106, The keyframe names borderShift and emojiFloat violate the kebab-case naming convention flagged by stylelint; rename these keyframes to kebab-case (e.g., border-shift and emoji-float) and update every reference to them (animation, animation-name, or shorthand usage) throughout the CSS so selectors and animations continue to work; ensure you change both the `@keyframes` declarations and all places that reference borderShift and emojiFloat (including the occurrences noted around lines 459-469).static/css/auth-modal.css (2)
245-255: Use centralized color variables frombase.css.The file hardcodes colors that are defined as CSS variables in
base.css:
#ef4444should usevar(--color-danger)#22c55eshould usevar(--color-success)This would ensure consistency with the new design system and make future color changes easier.
♻️ Proposed fix for error and strength indicator colors
/* Error message */ `#authModal` .auth-error { display: none; - color: `#ef4444`; + color: var(--color-danger); font-size: 14px; text-align: center; margin: 20px 20px 0 20px; padding: 8px 12px; - background: rgba(239, 68, 68, 0.1); - border: 1px solid rgba(239, 68, 68, 0.3); + background: color-mix(in srgb, var(--color-danger) 10%, transparent); + border: 1px solid color-mix(in srgb, var(--color-danger) 30%, transparent); border-radius: 8px; } `#authModal` .password-strength-bar { height: 100%; width: 0%; - background: `#ef4444`; + background: var(--color-danger); border-radius: 2px; transition: all 0.3s ease; } `#authModal` .password-strength-label { font-size: 12px; - color: `#ef4444`; + color: var(--color-danger); text-align: right; font-weight: 500; } `#authModal` .password-requirements .requirement-missing { - color: `#ef4444`; + color: var(--color-danger); } `#authModal` .password-requirements .requirement-missing .requirement-icon { - color: `#ef4444`; + color: var(--color-danger); } `#authModal` .password-requirements .requirement-met { - color: `#22c55e`; + color: var(--color-success); } `#authModal` .password-requirements .requirement-met .requirement-icon { - color: `#22c55e`; + color: var(--color-success); }Note: The
color-mix()function requires modern browser support. Alternatively, keep the rgba values but add a comment noting they should match--color-danger.Also applies to: 272-285, 310-324
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/css/auth-modal.css` around lines 245 - 255, Replace hardcoded color literals with the centralized CSS variables from base.css: change occurrences of "#ef4444" to "var(--color-danger)" and "#22c55e" to "var(--color-success)" in the auth modal styles (e.g., selector "#authModal .auth-error" and the password strength/indicator selectors that use those hex colors). For translucent backgrounds or borders that currently use rgba(...) derived from those hex values, either use color-mix() to derive an rgba from the variable (if acceptable) or keep the rgba but add a comment noting it should match var(--color-danger)/var(--color-success) for future refactors. Ensure all instances of these hex values across the file are replaced or annotated for consistency with the design system.
30-40: Optional: Consider kebab-case for keyframe name.Stylelint flags
authModalSlideInas not following kebab-case convention. Consider renaming toauth-modal-slide-infor consistency with CSS naming conventions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/css/auth-modal.css` around lines 30 - 40, Rename the keyframe identifier authModalSlideIn to kebab-case auth-modal-slide-in and update all references to it (e.g., any animation, animation-name or `@apply` usages) so the animation continues to work; specifically update the `@keyframes` declaration named authModalSlideIn to auth-modal-slide-in and change any occurrences of authModalSlideIn in the CSS classes or rules that set animation/animation-name to auth-modal-slide-in.static/css/confetti.css (1)
10-10: Consider using the new z-index CSS variable.The PR introduces a z-index scale in
base.css(--z-dropdown: 100, etc.). This hardcodedz-index: 11appears intentionally low (below UI elements), but for consistency with the new design system, consider whether this should reference a variable or be documented as intentionally below the scale.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/css/confetti.css` at line 10, The confetti CSS uses a hardcoded z-index: 11 which conflicts with the new z-index scale; update the declaration in static/css/confetti.css to use the design-system variable (e.g. replace z-index: 11 with z-index: var(--z-some-appropriate-key) such as var(--z-dropdown) or another lower-level token), or if 11 is intentionally below the scale, add an inline comment next to the z-index in the confetti rule documenting that it must remain below the z-index scale; reference the z-index declaration in the confetti stylesheet when making the change.static/css/landing-edit-modal.css (1)
35-35: Add a fallback for--z-maxto prevent potential styling issues if the CSS is ever used outside the current template context.While
base.cssis currently loaded beforelanding-edit-modal.cssin all uses, adding a fallback value makes the modal self-contained and more resilient to future refactoring.Suggested change
- z-index: var(--z-max) !important; + z-index: var(--z-max, 9999) !important;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/css/landing-edit-modal.css` at line 35, The z-index declaration in landing-edit-modal.css uses var(--z-max) without a fallback; update the rule that currently contains "z-index: var(--z-max) !important;" to provide a sensible fallback value (e.g., a large z-index like 9999) so the modal remains correctly stacked when --z-max is not defined; locate the same declaration in static/css/landing-edit-modal.css and replace it with a var() that includes the fallback.templates/legal/base.html (1)
42-42: Hardcoded z-index inconsistent with the new CSS variable system.This PR introduces centralized z-index CSS variables (
--z-modal,--z-sticky,--z-overlay, etc.) used throughout the dashboard and other stylesheets. The inlinestyle="z-index: 100;"here should be replaced with the appropriate CSS variable for consistency.Consider moving this to the
mobile-header.cssstylesheet usingvar(--z-sticky)or similar.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/legal/base.html` at line 42, The inline hardcoded z-index on the mobile navbar (the div with class "mobile-navbar") should be removed and replaced with the centralized CSS variable system; remove style="z-index: 100;" from the element and add a rule in mobile-header.css (or the appropriate stylesheet) setting .mobile-navbar { z-index: var(--z-sticky); } (or another appropriate variable such as --z-overlay/--z-modal), ensuring consistency with the project's z-index variables.templates/partials/analytics.html (1)
3-17: Don't wait forwindow.loadto bootstrap Sentry and Clarity.Delaying these SDKs until the full page load means Sentry misses startup errors and Clarity misses the first interactions. The injected scripts are already async, so it's better to bootstrap them immediately from the partial.
📈 Suggested fix
- addEventListener("load", function () { + (function () { {% if sentry_client_key %} var s = document.createElement("script"); - s.src = "https://js.sentry-cdn.com/{{ sentry_client_key }}.min.js"; + s.src = "https://js.sentry-cdn.com/" + {{ sentry_client_key | tojson }} + ".min.js"; + s.async = true; s.crossOrigin = "anonymous"; document.head.appendChild(s); {% endif %} {% if clarity_id %} (function (c, l, a, r, i, t, y) { c[a] = c[a] || function () { (c[a].q = c[a].q || []).push(arguments) }; t = l.createElement(r); t.async = 1; t.src = "https://www.clarity.ms/tag/" + i; y = l.getElementsByTagName(r)[0]; y.parentNode.insertBefore(t, y); - })(window, document, "clarity", "script", "{{ clarity_id }}"); + })(window, document, "clarity", "script", {{ clarity_id | tojson }}); {% endif %} - }); + })();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/partials/analytics.html` around lines 3 - 17, The analytics partial currently delays bootstrapping Sentry and Clarity by wrapping them in addEventListener("load", ...) which causes missed startup errors and early interactions; move the Sentry and Clarity injection logic out of the load handler so it runs immediately when the partial is rendered (keep the existing sentry_client_key and clarity_id conditionals), i.e., create and append the Sentry script element (s, document.head.appendChild) and immediately execute the Clarity IIFE (the function that assigns c[a] and inserts t) without waiting for window.load so the scripts still load async but start as soon as possible.static/js/auth-utils.js (1)
6-14: Don't force JSON headers in a shared fetch helper.
authFetchis now a generic dashboard utility, but hard-codingContent-Type: application/jsonon every request prevents the browser from supplying the correct header forFormData,URLSearchParams, and other non-JSON bodies. It's safer to let callers opt into JSON.♻️ Suggested fix
async function authFetch(url, options = {}) { - const defaultOptions = { - credentials: 'include', - headers: { - 'Content-Type': 'application/json', - ...options.headers - } - }; - const response = await fetch(url, { ...defaultOptions, ...options }); + const response = await fetch(url, { + credentials: 'include', + ...options + }); if (response.status === 401) { window.location.href = '/'; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/js/auth-utils.js` around lines 6 - 14, The authFetch helper currently forces 'Content-Type: application/json' via defaultOptions which breaks non-JSON bodies (FormData, URLSearchParams); update authFetch so defaultOptions does not include a forced Content-Type header (remove 'Content-Type' from defaultOptions.headers) and only let callers provide headers via options.headers (or set Content-Type conditionally in callers); refer to the authFetch function and the defaultOptions object when making this change so the fetch call merges caller-provided headers without overwriting them with application/json.static/js/dashboard/settings.js (1)
735-744: UseaddEventListenerinstead of assigning towindow.onclick.Directly assigning to
window.onclickoverwrites any previously registered handler. If another script also useswindow.onclick, one will be lost. UseaddEventListenerfor safer event binding.♻️ Proposed fix
-// Close modal when clicking outside -window.onclick = function (event) { +// Close modal when clicking outside +window.addEventListener('click', function (event) { const modals = document.querySelectorAll('.modal'); modals.forEach(modal => { if (event.target === modal) { const modalId = modal.id; closeModal(modalId); } }); -} +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/js/dashboard/settings.js` around lines 735 - 744, Replace the direct assignment to window.onclick with window.addEventListener('click', ...) to avoid overwriting other handlers; keep the existing logic that queries document.querySelectorAll('.modal') and compares event.target to each modal, then calls closeModal(modalId) when matched, but register it via addEventListener so multiple click handlers can coexist and the behavior remains the same.static/css/dashboard/settings.css (1)
482-497: Duplicate.btnclass definition with conflicting values.There's an earlier
.btndefinition at lines 97-109 withpadding: 6px 12px, but this one at lines 482-497 usespadding: 10px 16px. The second definition overrides the first due to CSS cascade, making the earlier one effectively dead code.Consider consolidating these into a single
.btndefinition, or use distinct class names (e.g.,.btn-modal) if different padding is intentional for different contexts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/css/dashboard/settings.css` around lines 482 - 497, There are two conflicting .btn rules; consolidate or namespace them: remove the duplicate .btn block or merge its differing properties into the original .btn (ensuring desired padding/height/gap are the final values), and if the larger padding is intentional for a specific context create and apply a new class name (e.g., .btn-modal or .btn--large) instead of redefining .btn; update usages in markup to use the new class if created.static/js/dashboard/links.js (1)
424-445: Consider extractingshowFieldErrorto a shared utility.This function is duplicated in
settings.jswith slight variations (element type, HTML handling). As part of the frontend rearchitecture, consolidating these into a shared module would improve maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/js/dashboard/links.js` around lines 424 - 445, The showFieldError function is duplicated across files; extract a single shared utility (e.g., export function showFieldError from a new frontend utility module) and update both static/js/dashboard/links.js and settings.js to import and use that centralized function; ensure the consolidated showFieldError handles both input/element types and message insertion modes used in settings.js (textContent vs innerHTML) or accept a flag/HTML-safe content option, preserve existing class names ('field-error' and 'error') and parentNode insertion behavior, and remove the duplicate implementations from the individual files so both use the shared exported function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@static/css/contacts-modal.css`:
- Line 9: contacts-modal.css uses the CSS variable --z-modal for z-index but
templates legal/base.html and templates/api.html include contacts-modal.css
without loading base.css where --z-modal is defined; update both templates
(legal/base.html and api.html) to add a link to base.css (i.e. <link
rel="stylesheet" href="{{ url_for('static', path='css/base.css') }}">) before
the link that includes contacts-modal.css so the --z-modal variable is defined
when contacts-modal.css is parsed.
In `@static/css/dashboard/settings.css`:
- Around line 537-552: The .btn-settings rule declares font-size twice; remove
the first font-size: 12px declaration so only font-size: 13px remains; update
the .btn-settings CSS block (look for the .btn-settings selector) to eliminate
the duplicate property and keep the intended font-size.
In `@static/css/header.css`:
- Line 10: The header.css uses CSS variables --z-max and --z-overlay that can be
undefined on api and legal routes; update the z-index declarations to provide
fallback numeric values (e.g., use var(--z-max, 9999) and var(--z-overlay, 9000)
wherever --z-max or --z-overlay are referenced) so header stacking works even
when base.css isn’t loaded; search for occurrences of "var(--z-max" and
"var(--z-overlay" in header.css (and any related selectors) and replace them
with var(..., <sane-fallback>) variants.
In `@static/css/v2-announcement.css`:
- Around line 68-74: The CSS `mask` shorthand is overriding `mask-composite`
(and -webkit-mask-composite) in the block containing -webkit-mask,
-webkit-mask-composite, mask-composite, and mask; fix by reordering so the
shorthand `mask` (and `-webkit-mask`) appear before the longhands
`mask-composite`/`-webkit-mask-composite`, or replace the `mask` shorthand with
`mask-image`/`-webkit-mask-image` so `mask-composite: exclude` remains
effective.
In `@static/js/dashboard/links.js`:
- Line 323: The condition is wrong because `!qrImg.naturalWidth === 0` always
evaluates to false; change the guard to explicitly check for a missing element
or a zero naturalWidth (e.g., replace the expression with a check like `!qrImg
|| qrImg.naturalWidth === 0`) so the function returns early when `qrImg` is
null/undefined or the image hasn't loaded; update the conditional around the
`qrImg` variable accordingly.
- Around line 1003-1018: There are two definitions of initSegments() causing
double initialization and missing button-active toggling; remove the duplicate
function (the one that defines initSegments with the querySelectorAll('.seg')
starting at lines ~1003) so only the original, full-featured initSegments() (the
one that also toggles button active classes) remains and keep its single
invocation; ensure no other calls reference the removed duplicate name and that
the retained initSegments handles setting hidden.value, data-active, and button
active class state.
In `@static/js/dashboard/settings.js`:
- Around line 720-725: The current showFieldError behavior uses
errorEl.innerHTML when message.includes('<'), which risks XSS for API-sourced
strings (e.g., data.error passed from the server); change showFieldError to
never set innerHTML from untrusted input—always use errorEl.textContent for
messages coming from the API (or sanitize before assigning). If you need to
render trusted HTML (like the hardcoded password requirements), create a
separate function such as showFieldErrorHTML and call it only with known-safe
hardcoded content; ensure showFieldError and any callers (e.g., the code that
passes data.error) use the safe textContent path.
- Line 204: The onclick currently uses eval(provider.action) which is insecure;
replace it with a safe lookup and direct call: create an action map object
(e.g., actionMap) that maps the expected action keys/strings (the ones stored in
provider.action, like "linkGoogleAccount" or "unlinkGitHubAccount") to the
actual functions (linkGoogleAccount, unlinkGitHubAccount, etc.), then change the
handler to lookup the function from actionMap using the provider.action key,
verify it is a function, and call it (e.g., const fn =
actionMap[provider.action]; if (typeof fn === 'function') fn()); ensure the
target functions (linkGoogleAccount, ...) are in scope and only expected keys
are allowed to avoid executing arbitrary code.
- Around line 684-700: The function validatePassword is dead code and should be
removed; delete the validatePassword function definition from the file and
ensure there are no remaining references to it, relying on the existing
validateAuthPassword for password validation instead; confirm no tests or
callers expect validatePassword and remove any related comments or exports if
present to avoid unused-symbol warnings.
In `@static/js/notifications.js`:
- Around line 24-40: The created notification div (variable "notification" in
notifications.js) is not announced to screen readers; mark it as a live region
by adding accessible attributes to the element: set role="status" (or
role="alert" for urgent errors), aria-live="polite" (or "assertive" for critical
submission errors), and aria-atomic="true" on the notification node so
screen-readers announce the whole message; ensure these attributes are added
before appending the node to document.body (target the notification variable
where className is set).
In `@templates/contact.html`:
- Around line 31-34: Inline JS uses server variables error and success in calls
to showNotification; use the Jinja tojson filter to safely JSON-serialize those
values before injecting into the script. Update the template locations where
showNotification("{{ error }}", ...) and showNotification("{{ success }}", ...)
are used to pass JSON-serialized values (e.g., {{ error|tojson }} and {{
success|tojson }}) so quotes, newlines, and backslashes are escaped correctly
for JavaScript.
In `@templates/index.html`:
- Around line 45-49: The template injects the raw error string into JavaScript
(showNotification call) which is unsafe; update the templates to serialize the
variable for JavaScript context by applying Jinja2's tojson filter when passing
the error to showNotification (and do the same for the other unsafe variables in
templates/stats.html, templates/report.html, and templates/contact.html), so
that showNotification receives a safely escaped JS value rather than an
interpolated HTML-escaped string.
In `@templates/report.html`:
- Around line 30-31: The inline script currently injects HTML-escaped strings
into JS via "{{ error }}" and "{{ success }}", which can break showNotification
when messages contain quotes, ampersands or newlines; change the template to
serialize these values as JS literals (e.g. use the template engine's JSON
serializer like error|tojson and success|tojson) inside the existing {% if error
%} / {% if success %} conditionals so showNotification receives properly escaped
JS values.
In `@templates/stats.html`:
- Around line 32-34: The inline JS currently interpolates template variables
directly into showNotification calls (showNotification("{{ error }}", ...) and
showNotification("{{ password_error }}", ...)) which can break for special
characters; update both call sites to JSON-safe encode the messages (e.g., use
the template engine's tojson/escapejs filter) so the string literal is safely
quoted and escaped when passed to showNotification, keeping the same second and
third arguments.
---
Outside diff comments:
In `@static/css/contact.css`:
- Around line 1-13: The body rule currently only sets background-image (data
URI) so if the SVG fails to load white text may be unreadable; add a solid dark
fallback by inserting a background-color (e.g. background-color: `#040337`;)
before the existing background-image declaration in the body selector to ensure
readable contrast when the image is blocked or slow to load.
In `@static/css/dashboard/dateRangePicker.css`:
- Around line 220-236: The hover state currently uses the same color as the
default so there is no visual feedback; update the .apply-btn:hover rule to use
a darker (or higher-contrast) color variant rather than var(--accent-primary) —
for example switch to a darker token like var(--accent-primary-dark) or a
slightly darkened hex/rgb value so hovering shows clear feedback while
preserving existing .apply-btn styles and transitions.
---
Nitpick comments:
In `@static/css/auth-modal.css`:
- Around line 245-255: Replace hardcoded color literals with the centralized CSS
variables from base.css: change occurrences of "#ef4444" to
"var(--color-danger)" and "#22c55e" to "var(--color-success)" in the auth modal
styles (e.g., selector "#authModal .auth-error" and the password
strength/indicator selectors that use those hex colors). For translucent
backgrounds or borders that currently use rgba(...) derived from those hex
values, either use color-mix() to derive an rgba from the variable (if
acceptable) or keep the rgba but add a comment noting it should match
var(--color-danger)/var(--color-success) for future refactors. Ensure all
instances of these hex values across the file are replaced or annotated for
consistency with the design system.
- Around line 30-40: Rename the keyframe identifier authModalSlideIn to
kebab-case auth-modal-slide-in and update all references to it (e.g., any
animation, animation-name or `@apply` usages) so the animation continues to work;
specifically update the `@keyframes` declaration named authModalSlideIn to
auth-modal-slide-in and change any occurrences of authModalSlideIn in the CSS
classes or rules that set animation/animation-name to auth-modal-slide-in.
In `@static/css/confetti.css`:
- Line 10: The confetti CSS uses a hardcoded z-index: 11 which conflicts with
the new z-index scale; update the declaration in static/css/confetti.css to use
the design-system variable (e.g. replace z-index: 11 with z-index:
var(--z-some-appropriate-key) such as var(--z-dropdown) or another lower-level
token), or if 11 is intentionally below the scale, add an inline comment next to
the z-index in the confetti rule documenting that it must remain below the
z-index scale; reference the z-index declaration in the confetti stylesheet when
making the change.
In `@static/css/dashboard/settings.css`:
- Around line 482-497: There are two conflicting .btn rules; consolidate or
namespace them: remove the duplicate .btn block or merge its differing
properties into the original .btn (ensuring desired padding/height/gap are the
final values), and if the larger padding is intentional for a specific context
create and apply a new class name (e.g., .btn-modal or .btn--large) instead of
redefining .btn; update usages in markup to use the new class if created.
In `@static/css/landing-edit-modal.css`:
- Line 35: The z-index declaration in landing-edit-modal.css uses var(--z-max)
without a fallback; update the rule that currently contains "z-index:
var(--z-max) !important;" to provide a sensible fallback value (e.g., a large
z-index like 9999) so the modal remains correctly stacked when --z-max is not
defined; locate the same declaration in static/css/landing-edit-modal.css and
replace it with a var() that includes the fallback.
In `@static/css/stats-view.css`:
- Around line 354-398: The `@keyframes` fade-in definition is overly verbose;
replace the multi-step opacity stops with a minimal equivalent (e.g., just 0% {
opacity: 0; } and 100% { opacity: 1; }) so the browser will interpolate the
in-between values automatically; update the `@keyframes` named fade-in accordingly
to keep the same visual effect while simplifying the CSS.
In `@static/css/v2-announcement.css`:
- Around line 94-106: The keyframe names borderShift and emojiFloat violate the
kebab-case naming convention flagged by stylelint; rename these keyframes to
kebab-case (e.g., border-shift and emoji-float) and update every reference to
them (animation, animation-name, or shorthand usage) throughout the CSS so
selectors and animations continue to work; ensure you change both the `@keyframes`
declarations and all places that reference borderShift and emojiFloat (including
the occurrences noted around lines 459-469).
In `@static/js/auth-utils.js`:
- Around line 6-14: The authFetch helper currently forces 'Content-Type:
application/json' via defaultOptions which breaks non-JSON bodies (FormData,
URLSearchParams); update authFetch so defaultOptions does not include a forced
Content-Type header (remove 'Content-Type' from defaultOptions.headers) and only
let callers provide headers via options.headers (or set Content-Type
conditionally in callers); refer to the authFetch function and the
defaultOptions object when making this change so the fetch call merges
caller-provided headers without overwriting them with application/json.
In `@static/js/dashboard/links.js`:
- Around line 424-445: The showFieldError function is duplicated across files;
extract a single shared utility (e.g., export function showFieldError from a new
frontend utility module) and update both static/js/dashboard/links.js and
settings.js to import and use that centralized function; ensure the consolidated
showFieldError handles both input/element types and message insertion modes used
in settings.js (textContent vs innerHTML) or accept a flag/HTML-safe content
option, preserve existing class names ('field-error' and 'error') and parentNode
insertion behavior, and remove the duplicate implementations from the individual
files so both use the shared exported function.
In `@static/js/dashboard/settings.js`:
- Around line 735-744: Replace the direct assignment to window.onclick with
window.addEventListener('click', ...) to avoid overwriting other handlers; keep
the existing logic that queries document.querySelectorAll('.modal') and compares
event.target to each modal, then calls closeModal(modalId) when matched, but
register it via addEventListener so multiple click handlers can coexist and the
behavior remains the same.
In `@templates/dashboard/partials/verification_banner.html`:
- Line 27: Replace the hardcoded z-index value in the verification banner
partial with the design-system CSS variable: locate the z-index declaration in
verification_banner.html (the rule currently using "z-index: 100") and change it
to use var(--z-dropdown) so the banner follows the shared --z-dropdown token
from base.css.
In `@templates/legal/base.html`:
- Line 42: The inline hardcoded z-index on the mobile navbar (the div with class
"mobile-navbar") should be removed and replaced with the centralized CSS
variable system; remove style="z-index: 100;" from the element and add a rule in
mobile-header.css (or the appropriate stylesheet) setting .mobile-navbar {
z-index: var(--z-sticky); } (or another appropriate variable such as
--z-overlay/--z-modal), ensuring consistency with the project's z-index
variables.
In `@templates/partials/analytics.html`:
- Around line 3-17: The analytics partial currently delays bootstrapping Sentry
and Clarity by wrapping them in addEventListener("load", ...) which causes
missed startup errors and early interactions; move the Sentry and Clarity
injection logic out of the load handler so it runs immediately when the partial
is rendered (keep the existing sentry_client_key and clarity_id conditionals),
i.e., create and append the Sentry script element (s, document.head.appendChild)
and immediately execute the Clarity IIFE (the function that assigns c[a] and
inserts t) without waiting for window.load so the scripts still load async but
start as soon as possible.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9d735f1e-2854-4d5e-ad56-02a0e896980e
📒 Files selected for processing (56)
static/css/api.cssstatic/css/auth-modal.cssstatic/css/auth.cssstatic/css/base.cssstatic/css/confetti.cssstatic/css/contact.cssstatic/css/contacts-modal.cssstatic/css/customNotification.cssstatic/css/dashboard.cssstatic/css/dashboard/billing.cssstatic/css/dashboard/dashboard-base.cssstatic/css/dashboard/dateRangePicker.cssstatic/css/dashboard/keys.cssstatic/css/dashboard/links.cssstatic/css/dashboard/settings.cssstatic/css/dashboard/statistics.cssstatic/css/dashboard/url-modal.cssstatic/css/docs.cssstatic/css/error.cssstatic/css/header.cssstatic/css/landing-edit-modal.cssstatic/css/mobile-header.cssstatic/css/notifications.cssstatic/css/password.cssstatic/css/preview.cssstatic/css/prism-duotone-dark.cssstatic/css/report.cssstatic/css/result.cssstatic/css/stats-view.cssstatic/css/v2-announcement.cssstatic/js/auth-utils.jsstatic/js/customNotification.jsstatic/js/dashboard.jsstatic/js/dashboard/keys.jsstatic/js/dashboard/links.jsstatic/js/dashboard/settings.jsstatic/js/index-script.jsstatic/js/index-validate.jsstatic/js/notifications.jsstatic/js/url-manager.jstemplates/api.htmltemplates/base.htmltemplates/contact.htmltemplates/dashboard/base.htmltemplates/dashboard/billing.htmltemplates/dashboard/links.htmltemplates/dashboard/partials/verification_banner.htmltemplates/dashboard/settings.htmltemplates/dashboard/statistics.htmltemplates/index.htmltemplates/legal/base.htmltemplates/partials/analytics.htmltemplates/partials/auth_modal.htmltemplates/password.htmltemplates/report.htmltemplates/stats.html
💤 Files with no reviewable changes (7)
- static/css/preview.css
- static/css/password.css
- static/js/customNotification.js
- templates/partials/auth_modal.html
- static/css/dashboard.css
- static/css/customNotification.css
- static/js/dashboard.js
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
static/js/confetti.js (1)
1-65:⚠️ Potential issue | 🟡 Minorconfetti.js auto-fires particles on every result page load instead of being triggered on demand.
The
setIntervalat line 16 starts immediately when the script loads, causing ~1.2 seconds of confetti particles to rain on result.html every time the page is viewed. This appears to be unintended—confetti animations should fire only on specific user actions, not automatically.
result-script.jshas no dependency on this auto-fire behavior (it only handles QR code interactions and the copy button), so the auto-firing serves no purpose and wastes resources.Wrap the particle logic in a function and call it explicitly only when needed, or remove the
setIntervalandsetTimeoutentirely if confetti is not intended for this page.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/js/confetti.js` around lines 1 - 65, The script currently auto-starts confetti via the top-level setInterval/ setTimeout (symbols: setInterval, interval, setTimeout) causing particles on every page load; remove those top-level calls and instead expose a function (e.g., startConfetti) that encapsulates the interval setup and uses the existing createConfetti() and animate() helpers, and optionally return or expose a stopConfetti/clear function that clears the interval and stops generation; update any callers to invoke startConfetti() only when you want the effect.static/css/dashboard/statistics.css (1)
1249-1276:⚠️ Potential issue | 🟡 MinorFix the Stylelint violation before merge.
Line 1276 has an empty line before a declaration inside
.filters-dropdown, which matches the reporteddeclaration-empty-line-beforefailure.🧹 Proposed fix
animation: none !important; /* Remove desktop animation */ z-index: var(--z-max); - overflow-y: auto;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/css/dashboard/statistics.css` around lines 1249 - 1276, Remove the extra blank line inside the .filters-dropdown rule that triggers the declaration-empty-line-before Stylelint error so declarations are contiguous (e.g., ensure the empty line just before the "overflow-y: auto;" declaration is deleted); update the .filters-dropdown block accordingly and re-run Stylelint/formatting to confirm the rule passes.
🧹 Nitpick comments (10)
schemas/dto/responses/url.py (1)
200-213: Optional: constrainreasonto aLiteralto lock the contract.The docstring enumerates
'length' | 'format' | 'taken'as the only valid values, but the field is typedstr | Noneso callers (and OpenAPI consumers) can't rely on it. Tightening toLiteral["length", "format", "taken"] | Nonedocuments the contract in the schema and catches drift at the DTO boundary:- reason: str | None = Field( + reason: Literal["length", "format", "taken"] | None = Field( default=None, description="When unavailable: 'length', 'format', or 'taken'.", examples=["taken"], )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schemas/dto/responses/url.py` around lines 200 - 213, The reason field on AliasCheckResponse is typed too loosely (str | None) despite the docstring specifying only "length", "format", or "taken"; change the type of reason on the AliasCheckResponse DTO to a Literal union (Literal["length", "format", "taken"] | None) so the Pydantic/OpenAPI schema and type checker enforce the contract and prevent drift, update any imports to include typing.Literal where needed and run tests/type checks to confirm compatibility with ResponseBase.static/js/field-error.js (1)
79-87: Minor:Input should be.*catch-all drops useful detail.The last replacement flattens every Pydantic
Input should be …message (e.g. "Input should be a valid URL", "Input should be a valid integer", "Input should be greater than 0") to a bare"Invalid value". Users lose the actionable hint. Consider either keeping the tail verbatim (.replace(/^Input should be\s+/i, 'Must be ')) or whitelisting the common cases. The other replacers in this chain preserve detail; this one doesn't.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/js/field-error.js` around lines 79 - 87, The replacement that collapses all "Input should be …" messages to "Invalid value" removes useful detail; update the replace call in the msg transformation chain (the block that mutates the msg variable in static/js/field-error.js) to preserve the tail wording—for example change the regex from /^Input should be.*$/i to /^Input should\s+/i and replace with "Must be " so messages like "Input should be a valid URL" become "Must be a valid URL" (adjust wording to match surrounding style).static/css/dashboard/url-modal.css (2)
619-665: Theming inconsistency: base uses tokens, hover/active are hardcoded.
.btn-primary,.btn-warning, and.btn-dangernow resolve their base color viavar(--color-info | warning | danger), but their:hover/:activestill hardcode#5855eb,#4f46e5,#d97706,#b45309,#b91c1c. If those tokens are ever re-themed (dark mode variant, brand change) the buttons will desync on interaction.btn-danger:hoveralready uses--color-danger-hover— worth following that pattern for the others (e.g.--color-info-hover,--color-info-active, etc.) or deriving viacolor-mix().Not a blocker for this PR — follow-up.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/css/dashboard/url-modal.css` around lines 619 - 665, The hover/active states for .btn-primary and .btn-warning are hardcoded while the base uses CSS tokens, causing theme drift; update the :hover and :active rules for .btn-primary, .btn-warning (and keep .btn-danger pattern) to use CSS variables (e.g. --color-info-hover, --color-info-active, --color-warning-hover, --color-warning-active) or derive via color-mix() so interactive states follow theming, and ensure you reference the same token naming convention already used by .btn-danger (:hover uses --color-danger-hover) when editing the selectors .btn-primary:hover, .btn-primary:active, .btn-warning:hover, and .btn-warning:active.
354-360: Minor: inconsistent fallback on status colors.
.alias-status.is-availableusesvar(--color-success,#22c55e)with a literal fallback, but.alias-status.is-unavailableusesvar(--color-danger)bare. Either both should keep a fallback or neither — otherwise a stylesheet load order issue (e.g.base.cssnot yet evaluated) produces the success color but an unstyled danger state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/css/dashboard/url-modal.css` around lines 354 - 360, The CSS uses a literal fallback for .alias-status.is-available (var(--color-success, `#22c55e`)) but no fallback for .alias-status.is-unavailable (var(--color-danger)), causing inconsistent behavior if CSS variables are not loaded; update the .alias-status.is-unavailable rule to include a sensible fallback (e.g., var(--color-danger, `#ef4444`)) so both status rules have explicit fallbacks and behave consistently even if base variables are missing.templates/dashboard/partials/sidebar.html (1)
64-65: Add initialaria-expandedandaria-haspopupon the dropdown trigger.
dropdown.jssetsaria-expandeddynamically on open/close, but without an initialaria-expanded="false"on first render screen readers won't know this is a collapsible menu.aria-haspopup="menu"andaria-controls="profileMenu"also help assistive tech. Minor a11y polish.- <button type="button" class="profile-button dropdown-trigger" id="profileButton" - data-dropdown-trigger aria-label="Profile menu"> + <button type="button" class="profile-button dropdown-trigger" id="profileButton" + data-dropdown-trigger aria-label="Profile menu" + aria-haspopup="menu" aria-expanded="false" aria-controls="profileMenu">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/dashboard/partials/sidebar.html` around lines 64 - 65, Add initial ARIA attributes to the dropdown trigger button: set aria-expanded="false", aria-haspopup="menu", and aria-controls="profileMenu" on the element with id "profileButton" so screen readers know it's a collapsible menu; keep dropdown.js behavior that toggles aria-expanded on open/close to update the state dynamically.static/css/dashboard/keys.css (2)
1207-1218: Inconsistent with the rest of the refactor: replace hardcoded hex with the new tokens.This PR centralizes status colors via CSS variables, but the delete-icon gradient here still uses raw
#ef4444,#dc2626`` — the exact values of--color-danger/ `--color-danger-hover` defined in `base.css`. Swap them for the variables so the theme stays consistent (and automatically updates if the palette is tweaked later).- background: linear-gradient(135deg, `#ef4444`, `#dc2626`); + background: linear-gradient(135deg, var(--color-danger), var(--color-danger-hover));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/css/dashboard/keys.css` around lines 1207 - 1218, The gradient for the delete icon is using hardcoded hex colors; update the selector `#delete-key-modal` .modal-icon to use the centralized CSS variables instead of raw values by replacing the linear-gradient stops with var(--color-danger) and var(--color-danger-hover) (or the exact token names used in base.css) so the component follows the refactor and theme changes propagate.
1143-1294: Consider promoting the delete-confirm modal to a shared primitive.The comment on line 1143 —
(duplicated from links url-modal delete styles)— acknowledges the duplication. Since the dashboard now has (at least) two copies of essentially the same "delete confirmation" modal CSS, extracting these ~150 lines into a sharedmodal-confirm-deleteprimitive (e.g. instatic/css/dashboard/url-modal.cssor a newmodal-confirm.css) would prevent drift and keep future tweaks in one place. Fine to defer, but worth filing a follow-up.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/css/dashboard/keys.css` around lines 1143 - 1294, Duplicate delete-confirm modal CSS exists (`#delete-key-modal` and its nested selectors like .modal-container, .modal-content, .modal-header, .modal-body, .modal-footer, .btn-cancel, .btn-delete); extract these shared rules into a new shared stylesheet (e.g., modal-confirm-delete or modal-confirm.css) and replace the duplicate block with a single import or include so both the keys delete modal and the links/url-modal reuse the same classes/selectors; ensure selector names remain identical (or convert to a common class like .modal-confirm) and update markup references if you rename selectors so both components use the shared primitive.templates/dashboard/base.html (1)
60-65: Considerdeferfor the base scripts block.These are all sync
<script>tags in<body>and run blocking in order. Since they only need to run after the DOM exists and have a strict ordering dependency (dropdown.jsbeforedashboard-base.js,notifications.jsbefore the inline verified handler, etc.), addingdeferto each would preserve order while allowing parallel download and non-blocking parse. Optional perf improvement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/dashboard/base.html` around lines 60 - 65, The script tags in the base scripts block are currently synchronous and block parsing; update each <script> tag that loads notifications.js, field-error.js, dropdown.js, sheet-handle.js, modal-tabs.js and dashboard/dashboard-base.js in templates/dashboard/base.html to include the defer attribute so they download in parallel but execute in order after DOM parsing, preserving the existing dependency ordering (e.g. dropdown.js before dashboard-base.js, notifications.js before any inline verified-handler).routes/api_v1/shorten.py (1)
85-102: LGTM on the endpoint shape; one small consideration.The handler is clean and rate-limiting/auth wiring is consistent with the existing
shorten_v1pattern. One thought: since the response reflects live DB state and the UI polls it on keystrokes, consider settingCache-Control: no-storeon the response so intermediaries/CDNs don't ever cache a stale{available: true}that later becomes false. Optional.Note: the root-cause correctness concern for this endpoint (emoji aliases being reported as
"format") is flagged upstream onUrlService.check_aliasinservices/url_service.py.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@routes/api_v1/shorten.py` around lines 85 - 102, The endpoint check_alias should set Cache-Control: no-store to prevent intermediaries/CDNs caching live availability results; add a fastapi Response parameter (e.g., response: Response) to the check_alias signature and set response.headers["Cache-Control"] = "no-store" before returning the AliasCheckResponse (keep UrlSvc/UrlService.check_alias usage unchanged).static/js/dashboard/links.js (1)
117-160: Duplicated copy-success UI logic in.then/.catchbranches.The same ~15 lines that toggle
.copied, swap icon classes, and schedule the revert are repeated in the success and fallback paths here, and again incopyCreatedLinkToClipboard(lines 241–272). Consider extracting a small helper such asflashCopyButtonCopied(btn)to reduce drift between the two callsites.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/js/dashboard/links.js` around lines 117 - 160, Duplicate UI update logic for the copy button should be extracted into a helper to avoid drift: create a small function named flashCopyButtonCopied(buttonOrId) (or similar) that finds the element (or accepts the element), adds the 'copied' class, swaps icon classes to 'ti ti-check', and schedules reverting the class and icon after 2s; replace the repeated blocks in the navigator.clipboard.writeText.then, its catch fallback, and the existing copyCreatedLinkToClipboard function to call flashCopyButtonCopied(document.getElementById('btn-copy-created-link')) instead of duplicating the DOM manipulation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/url_service.py`:
- Around line 290-304: check_alias currently marks pure-emoji inputs as "format"
because it only calls validate_alias; update it to mirror create() by accepting
aliases that pass either validate_alias or validate_emoji_alias. In the
check_alias function, after the length check call validate_emoji_alias in
addition to validate_alias and only return "format" if neither validator passes;
keep the existing availability check (check_alias_available) and return values
("length", "format", "taken", "available") unchanged so AliasCheckResult aligns
with create().
In `@static/css/dashboard/keys.css`:
- Around line 101-104: The hover rule for .btn-primary:hover currently sets
background and border-color to the same var(--accent-primary) as the base state,
removing hover feedback; update the selector (.btn-primary:hover) to use a
distinct hover token (e.g., var(--accent-primary-hover)) or a subtly darker
color so the hover is visually distinguishable from .btn-primary, and add the
new CSS variable definition where theme vars are declared (or compute a darker
variant) so both background and border-color change on hover.
In `@static/css/dashboard/statistics.css`:
- Around line 1821-1822: The project currently suppresses AnyChart credits by
CSS (.anychart-credits { display: none !important; }) while loading AnyChart
without a license; fix this by either removing the CSS rules that hide
.anychart-credits (remove the rule from the stylesheets that contain it) or by
configuring a valid AnyChart license via anychart.licenseKey() before anychart
chart initialization (add a script call to anychart.licenseKey(YOUR_KEY) in the
pages that load charts—templates that initialize charts like the
statistics/stats view templates—ensuring the key is injected securely from
server/config rather than hard-coding). Ensure the chosen approach is applied
consistently where .anychart-credits appears and before anychart chart code
runs.
In `@static/js/dashboard/links.js`:
- Around line 99-112: The QR image flow lacks an onerror handler causing the
qr-loading animation and opacity=0 to persist if the remote QR fails; update the
code around the qrImg usage to add qrImg.onerror that clears
qrImg.parentElement.style.animation, restores qrImg.style.opacity (or hides the
image and shows a fallback/placeholder), and optionally calls
setupQRCodeInteractions fallback logic or sets a local placeholder src so users
get feedback; ensure the handler is assigned before setting qrImg.src and mirror
the same cleanup that onload performs (stopping animation) while surfacing a
visible fallback state.
- Around line 716-719: The toEpochSeconds function currently lets Invalid Date
produce NaN (try/catch is ineffective); change it to explicitly validate the
timestamp: create const ts = new Date(value).getTime(); if (!value || isNaN(ts))
return undefined; otherwise return Math.floor(ts / 1000). Update the
toEpochSeconds implementation (used by buildQuery) so invalid inputs produce
undefined instead of NaN/null in serialized filters.
In `@static/js/dropdown.js`:
- Around line 183-195: The init() function currently assigns role="menu" to
every dropdown panel via getMenu() which incorrectly applies menu semantics to
form-style panels that contain inputs/selects; change the logic in init() so
that after retrieving menu = getMenu(d) you only set
menu.setAttribute('role','menu') when the panel is an action menu (e.g., detect
via a CSS class like '.dropdown--menu' or an ARIA/data attribute on the dropdown
element) and skip setting the role for panels that are form/option panels (those
without the marker); update any documentation or markup expectation so callers
can mark true action menus accordingly.
In `@static/js/url-manager.js`:
- Around line 6-12: The normalizeUrl function currently only checks for http(s)
and will prepend https:// to inputs like ftp://..., corrupting them; update
normalizeUrl to first detect any existing URL scheme (e.g. using a regex for
scheme like /^[a-zA-Z][a-zA-Z0-9+.-]*:\/\//i) and if a scheme exists return the
trimmed input unchanged, otherwise prepend 'https://' as before; reference the
normalizeUrl function and replace the current /^https?:\/\//i check with the
broader scheme-detection check.
In `@templates/dashboard/keys.html`:
- Around line 247-276: The new delete-key modal (`#delete-key-modal`) is missing
standard ARIA attributes; update the <div id="delete-key-modal"> to include
role="dialog", aria-modal="true", and aria-hidden="true" and add aria-labelledby
that points to a unique id on the modal title (e.g., give the <h2
class="modal-title"> a new id like modal-title-delete and reference it from
aria-labelledby); mirror the same attribute pattern used by `#createKeyModal` and
`#keySuccessModal` so assistive tech will announce and treat this as a modal
dialog for the destructive confirmation.
---
Outside diff comments:
In `@static/css/dashboard/statistics.css`:
- Around line 1249-1276: Remove the extra blank line inside the
.filters-dropdown rule that triggers the declaration-empty-line-before Stylelint
error so declarations are contiguous (e.g., ensure the empty line just before
the "overflow-y: auto;" declaration is deleted); update the .filters-dropdown
block accordingly and re-run Stylelint/formatting to confirm the rule passes.
In `@static/js/confetti.js`:
- Around line 1-65: The script currently auto-starts confetti via the top-level
setInterval/ setTimeout (symbols: setInterval, interval, setTimeout) causing
particles on every page load; remove those top-level calls and instead expose a
function (e.g., startConfetti) that encapsulates the interval setup and uses the
existing createConfetti() and animate() helpers, and optionally return or expose
a stopConfetti/clear function that clears the interval and stops generation;
update any callers to invoke startConfetti() only when you want the effect.
---
Nitpick comments:
In `@routes/api_v1/shorten.py`:
- Around line 85-102: The endpoint check_alias should set Cache-Control:
no-store to prevent intermediaries/CDNs caching live availability results; add a
fastapi Response parameter (e.g., response: Response) to the check_alias
signature and set response.headers["Cache-Control"] = "no-store" before
returning the AliasCheckResponse (keep UrlSvc/UrlService.check_alias usage
unchanged).
In `@schemas/dto/responses/url.py`:
- Around line 200-213: The reason field on AliasCheckResponse is typed too
loosely (str | None) despite the docstring specifying only "length", "format",
or "taken"; change the type of reason on the AliasCheckResponse DTO to a Literal
union (Literal["length", "format", "taken"] | None) so the Pydantic/OpenAPI
schema and type checker enforce the contract and prevent drift, update any
imports to include typing.Literal where needed and run tests/type checks to
confirm compatibility with ResponseBase.
In `@static/css/dashboard/keys.css`:
- Around line 1207-1218: The gradient for the delete icon is using hardcoded hex
colors; update the selector `#delete-key-modal` .modal-icon to use the centralized
CSS variables instead of raw values by replacing the linear-gradient stops with
var(--color-danger) and var(--color-danger-hover) (or the exact token names used
in base.css) so the component follows the refactor and theme changes propagate.
- Around line 1143-1294: Duplicate delete-confirm modal CSS exists
(`#delete-key-modal` and its nested selectors like .modal-container,
.modal-content, .modal-header, .modal-body, .modal-footer, .btn-cancel,
.btn-delete); extract these shared rules into a new shared stylesheet (e.g.,
modal-confirm-delete or modal-confirm.css) and replace the duplicate block with
a single import or include so both the keys delete modal and the links/url-modal
reuse the same classes/selectors; ensure selector names remain identical (or
convert to a common class like .modal-confirm) and update markup references if
you rename selectors so both components use the shared primitive.
In `@static/css/dashboard/url-modal.css`:
- Around line 619-665: The hover/active states for .btn-primary and .btn-warning
are hardcoded while the base uses CSS tokens, causing theme drift; update the
:hover and :active rules for .btn-primary, .btn-warning (and keep .btn-danger
pattern) to use CSS variables (e.g. --color-info-hover, --color-info-active,
--color-warning-hover, --color-warning-active) or derive via color-mix() so
interactive states follow theming, and ensure you reference the same token
naming convention already used by .btn-danger (:hover uses --color-danger-hover)
when editing the selectors .btn-primary:hover, .btn-primary:active,
.btn-warning:hover, and .btn-warning:active.
- Around line 354-360: The CSS uses a literal fallback for
.alias-status.is-available (var(--color-success, `#22c55e`)) but no fallback for
.alias-status.is-unavailable (var(--color-danger)), causing inconsistent
behavior if CSS variables are not loaded; update the
.alias-status.is-unavailable rule to include a sensible fallback (e.g.,
var(--color-danger, `#ef4444`)) so both status rules have explicit fallbacks and
behave consistently even if base variables are missing.
In `@static/js/dashboard/links.js`:
- Around line 117-160: Duplicate UI update logic for the copy button should be
extracted into a helper to avoid drift: create a small function named
flashCopyButtonCopied(buttonOrId) (or similar) that finds the element (or
accepts the element), adds the 'copied' class, swaps icon classes to 'ti
ti-check', and schedules reverting the class and icon after 2s; replace the
repeated blocks in the navigator.clipboard.writeText.then, its catch fallback,
and the existing copyCreatedLinkToClipboard function to call
flashCopyButtonCopied(document.getElementById('btn-copy-created-link')) instead
of duplicating the DOM manipulation.
In `@static/js/field-error.js`:
- Around line 79-87: The replacement that collapses all "Input should be …"
messages to "Invalid value" removes useful detail; update the replace call in
the msg transformation chain (the block that mutates the msg variable in
static/js/field-error.js) to preserve the tail wording—for example change the
regex from /^Input should be.*$/i to /^Input should\s+/i and replace with "Must
be " so messages like "Input should be a valid URL" become "Must be a valid URL"
(adjust wording to match surrounding style).
In `@templates/dashboard/base.html`:
- Around line 60-65: The script tags in the base scripts block are currently
synchronous and block parsing; update each <script> tag that loads
notifications.js, field-error.js, dropdown.js, sheet-handle.js, modal-tabs.js
and dashboard/dashboard-base.js in templates/dashboard/base.html to include the
defer attribute so they download in parallel but execute in order after DOM
parsing, preserving the existing dependency ordering (e.g. dropdown.js before
dashboard-base.js, notifications.js before any inline verified-handler).
In `@templates/dashboard/partials/sidebar.html`:
- Around line 64-65: Add initial ARIA attributes to the dropdown trigger button:
set aria-expanded="false", aria-haspopup="menu", and aria-controls="profileMenu"
on the element with id "profileButton" so screen readers know it's a collapsible
menu; keep dropdown.js behavior that toggles aria-expanded on open/close to
update the state dynamically.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 59222285-ea42-4ad3-9829-c22258bc4658
📒 Files selected for processing (31)
middleware/rate_limiter.pyroutes/api_v1/shorten.pyschemas/dto/requests/url.pyschemas/dto/responses/url.pyservices/url_service.pystatic/css/dashboard/apps.cssstatic/css/dashboard/dashboard-base.cssstatic/css/dashboard/dateRangePicker.cssstatic/css/dashboard/keys.cssstatic/css/dashboard/links.cssstatic/css/dashboard/statistics.cssstatic/css/dashboard/url-modal.cssstatic/css/dropdown.cssstatic/js/alias-checker.jsstatic/js/confetti.jsstatic/js/dashboard/dashboard-base.jsstatic/js/dashboard/dateRangePicker.jsstatic/js/dashboard/keys.jsstatic/js/dashboard/links.jsstatic/js/dashboard/statistics.jsstatic/js/dropdown.jsstatic/js/field-error.jsstatic/js/modal-tabs.jsstatic/js/sheet-handle.jsstatic/js/url-manager.jstemplates/dashboard/apps.htmltemplates/dashboard/base.htmltemplates/dashboard/keys.htmltemplates/dashboard/links.htmltemplates/dashboard/partials/sidebar.htmltemplates/dashboard/statistics.html
✅ Files skipped from review due to trivial changes (2)
- static/css/dropdown.css
- middleware/rate_limiter.py
🚧 Files skipped from review as they are similar to previous changes (4)
- static/css/dashboard/dateRangePicker.css
- static/js/dashboard/keys.js
- templates/dashboard/statistics.html
- static/css/dashboard/links.css
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
templates/base.html (1)
14-43:⚠️ Potential issue | 🟡 MinorVersion drift with
templates/legal/base.html— bump itsheader.css/header.jsto?v=7as well.
templates/base.htmlnow loadsheader.css?v=7(line 14) andheader.js?v=7(line 43), buttemplates/legal/base.htmlstill references both at?v=6. Because the self-promo close wiring was moved intostatic/js/header.js(and the oldself-promo.js/self-promo.csswere removed), users who visit a legal page with the oldheader.jscached will not get the close-button handler, and.self-promostyles that now live inheader.csswill also be stale. Bump those references intemplates/legal/base.htmlto keep cache-busting aligned across the app.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/base.html` around lines 14 - 43, Update templates/legal/base.html to align cache-busting with the main template by changing the header asset versions: bump the header CSS query param from ?v=6 to ?v=7 and the header JS query param from ?v=6 to ?v=7 so the file references match templates/base.html (this ensures the self-promo styles in header.css and the close-button wiring in header.js are loaded for legal pages).
♻️ Duplicate comments (4)
static/js/dashboard/links.js (3)
717-720:⚠️ Potential issue | 🟡 MinorReturn
undefinedfor invalid date filters.
new Date(value)does not throw for invalid input; it producesNaN, which then serializes asnullinside the filter JSON.🐛 Proposed fix
function toEpochSeconds(value) { if (!value) return undefined; - try { return Math.floor(new Date(value).getTime() / 1000); } catch { return undefined; } + const ms = new Date(value).getTime(); + return Number.isFinite(ms) ? Math.floor(ms / 1000) : undefined; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/js/dashboard/links.js` around lines 717 - 720, The helper toEpochSeconds currently uses new Date(value) which doesn't throw on invalid input and results in NaN that serializes as null; update toEpochSeconds to explicitly validate the created Date (e.g., const d = new Date(value); const t = d.getTime(); if (!value || Number.isNaN(t)) return undefined;) and otherwise return Math.floor(t/1000) so invalid/empty inputs produce undefined instead of NaN/null in the filter JSON.
99-111:⚠️ Potential issue | 🟡 MinorHandle QR image load failures.
If
qr.spoo.mefails, the loading animation remains and the QR stays invisible because onlyonloadclears the state.🛠️ Proposed fix
qrImg.onload = function () { qrImg.parentElement.style.animation = "none"; qrImg.style.opacity = "1"; setupQRCodeInteractions(); }; + qrImg.onerror = function () { + qrImg.parentElement.style.animation = "none"; + qrImg.style.opacity = "1"; + if (typeof showNotification === 'function') { + showNotification('Failed to load QR code', 'error'); + } + }; qrImg.src = `https://qr.spoo.me/api/v1/gradient?content=${enc}&size=280&start=%231d1919&end=%23322c29`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/js/dashboard/links.js` around lines 99 - 111, The QR image loading block only handles successful load via qrImg.onload, so failures leave the loading animation and invisible image; add an error handler (qrImg.onerror) that clears the parent animation, restores visibility (e.g., set qrImg.style.opacity = "1" or show a fallback), and calls or skips setupQRCodeInteractions as appropriate; ensure the same cleanup done in qrImg.onload (clearing parentElement.style.animation and adjusting opacity) is performed in qrImg.onerror for the element referenced by qrImg and avoid leaving dangling event handlers.
308-310:⚠️ Potential issue | 🔴 CriticalFix the QR loaded-state guard.
!qrImg.naturalWidth === 0negates the number before comparison, so it never checks for an unloaded image correctly.🐛 Proposed fix
function downloadQRCodeFromModal() { const qrImg = document.getElementById("created-link-qr"); - if (!qrImg || !qrImg.naturalWidth === 0) return; + if (!qrImg || qrImg.naturalWidth === 0) return;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/js/dashboard/links.js` around lines 308 - 310, The guard in downloadQRCodeFromModal incorrectly negates naturalWidth; update it to return early when the element is missing or the image hasn't loaded by checking the naturalWidth value directly (e.g., use a condition like "if (!qrImg || qrImg.naturalWidth === 0) return" or equivalently "if (!qrImg || !qrImg.naturalWidth) return"); this fixes the unloaded-image check for the element with id "created-link-qr" inside the downloadQRCodeFromModal function.static/js/dropdown.js (1)
156-168:⚠️ Potential issue | 🟠 MajorDon’t apply menu semantics to every dropdown panel.
This still assigns
aria-haspopup="menu"androle="menu"to all dropdowns, including option/filter panels that contain inputs/selects. Only true action menus should receive menu semantics; form-style panels should keep native form semantics.♿ Proposed fix
+ function isActionMenu(menu) { + return !!menu && ( + menu.matches('[data-dropdown-role="menu"], .dropdown-menu--actions') || + menu.querySelector('.dropdown-item') + ); + } + function init() { document.addEventListener('click', onDocumentClick); document.addEventListener('keydown', handleKeydown); // Prime aria attributes on existing dropdowns. document.querySelectorAll('.dropdown').forEach(function (d) { var trigger = getTrigger(d); + var menu = getMenu(d); if (trigger) { - trigger.setAttribute('aria-haspopup', 'menu'); + if (isActionMenu(menu)) trigger.setAttribute('aria-haspopup', 'menu'); trigger.setAttribute('aria-expanded', 'false'); } - var menu = getMenu(d); - if (menu) menu.setAttribute('role', 'menu'); + if (isActionMenu(menu)) menu.setAttribute('role', 'menu'); }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/js/dropdown.js` around lines 156 - 168, The init() function currently applies menu semantics to every .dropdown; change it so you only set aria-haspopup="menu" on the trigger (via getTrigger(d)) and role="menu" on the menu (via getMenu(d)) when the panel is a true action menu: detect this by checking for a marker (e.g., a data attribute like data-dropdown-type="menu" or an existing role) or by verifying the menu does NOT contain form controls (inputs, selects, textareas, buttons) before setting semantics; leave panels containing form controls untouched so they retain native form semantics. Ensure to update both the trigger.setAttribute('aria-haspopup','menu') / trigger.setAttribute('aria-expanded','false') and the menu.setAttribute('role','menu') calls to be conditional based on that check.
🧹 Nitpick comments (4)
templates/dashboard/settings.html (1)
6-6: Consider using Jinja'surl_forquery string or a build-based cache-busting token.The manual
?v=1suffix on the stylesheet (and?v=1/?v=2on scripts at lines 239-241) requires remembering to bump each version string on every file change. If a version bump is missed, users will be served stale cached assets. Consider centralizing this via a config value or file-hash helper so cache busting happens automatically.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/dashboard/settings.html` at line 6, The template currently appends manual cache-busters like "?v=1" to assets (e.g., the stylesheet link for "css/dashboard/settings.css" and the scripts referenced around lines 239–241); change this to a centralized solution by either using Jinja's url_for query argument (e.g., url_for('static', path='css/...', v=app.config['ASSET_VERSION'])) or replace the manual suffix with a helper that returns a file-hash or global version variable (config key like ASSET_VERSION or a template helper function get_asset_url) and use that for all asset references so cache-busting is automatic and consistent across settings.css and the referenced script tags.static/js/header.js (1)
139-149: Consider moving the self-promo close wiring inside the existingDOMContentLoadedhandler for consistency.Functionally fine because the script is loaded with
defereverywhere it's used, so the DOM is already parsed when this runs. But the rest of the file initializes DOM bindings inside theDOMContentLoadedcallback at line 72 — keeping this block with the others would make the setup consistent and more defensive should thedeferattribute ever be dropped on an include.♻️ Optional refactor
-}); - -const closeButton = document.querySelector('.self-promo-close'); - -if (closeButton) { - closeButton.addEventListener('click', closeSelfPromo); -} - -function closeSelfPromo() { - const promo = document.querySelector('.self-promo'); - if (promo) { - promo.classList.add('hidden'); - } -} + const closeButton = document.querySelector('.self-promo-close'); + if (closeButton) { + closeButton.addEventListener('click', function () { + const promo = document.querySelector('.self-promo'); + if (promo) promo.classList.add('hidden'); + }); + } +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/js/header.js` around lines 139 - 149, Move the self-promo wiring into the existing DOMContentLoaded handler: remove the top-level query/select and event hookup for closeButton and instead perform document.querySelector('.self-promo-close') and addEventListener('click', closeSelfPromo) inside the existing DOMContentLoaded callback (so the binding runs with the other DOM initializations); you can keep the closeSelfPromo function where it is or move its declaration into the same callback for locality, but ensure closeSelfPromo is defined before it's referenced.static/css/dashboard/keys.css (2)
1155-1305: Consider extracting the shared confirmation-modal styles.The comment on line 1155 explicitly calls this out: "duplicated from links url-modal delete styles". ~150 lines of modal scaffolding (container, backdrop, content transitions, header/body/footer, cancel/delete buttons, mobile overrides) are now copy-pasted across at least two pages. Any future tweak (padding, radius, transition curve, a11y focus ring) has to be made in N places and will drift.
Suggested path: hoist the shared pieces into a generic class such as
.confirm-modal(or.danger-confirm-modal) indashboard-base.css/a newmodals.css, and let#delete-key-modal/ the url-modal target it via an additional class. The id can stay for JS hooks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/css/dashboard/keys.css` around lines 1155 - 1305, Duplicate modal styling exists for `#delete-key-modal` and the links url-modal; extract shared styles into a reusable class (e.g., .confirm-modal or .danger-confirm-modal) in a common stylesheet and have `#delete-key-modal` simply add that class. Move shared selectors like .modal-container, .modal-content, .modal-header, .modal-body, .modal-footer, .modal-title-section-delete, .modal-icon, .modal-title, .btn-cancel and .btn-delete (including transitions, backdrop, sizing, padding, borders, and responsive rules) into the new shared class, keep only ID-specific overrides (colors or wording) in the existing `#delete-key-modal` block, and update the other modal (url-modal) to use the same shared class so JS can still target the element by ID while styles are centralized.
1186-1230: Finish the token migration inside#delete-key-modal.This PR's stated goal is to route danger/radius theming through shared tokens, but the new modal reintroduces hard-coded values:
- Line 1190:
border-radius: 16px— the rest of the file usesvar(--radius-lg).- Line 1228:
linear-gradient(135deg,#ef4444,#dc2626)— those are literally--color-danger/--color-danger-hover. If the theme tokens ever change, this icon will drift.Same applies (pre-existing, but aggravated by this PR) to
.btn-danger:hover(line 125) and the.status-active/-revoked/-expiredrgba backgrounds/borders (lines 279-293) that still use literalrgba(239, 68, 68, …)/rgba(34, 197, 94, …)/rgba(245, 158, 11, …)while theircolorwas switched to tokens — worth rounding out so the mapping is consistent.♻️ Suggested diff for the delete modal
`#delete-key-modal` .modal-content { backdrop-filter: blur(60px); background: rgba(15, 20, 35, 0.5); border: 1px solid rgba(255, 255, 255, 0.1); - border-radius: 16px; + border-radius: var(--radius-lg); @@ `#delete-key-modal` .modal-icon { ... - background: linear-gradient(135deg, `#ef4444`, `#dc2626`); + background: linear-gradient(135deg, var(--color-danger), var(--color-danger-hover)); color: white; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/css/dashboard/keys.css` around lines 1186 - 1230, Replace hard-coded radius and color literals with the theme tokens: in `#delete-key-modal` .modal-content swap border-radius: 16px for border-radius: var(--radius-lg); in `#delete-key-modal` .modal-icon replace linear-gradient(135deg, `#ef4444`, `#dc2626`) with linear-gradient(135deg, var(--color-danger), var(--color-danger-hover)) (or the hover token your theme uses). Also update other literal-danger/success/warning rgba usages referenced in the comment (.btn-danger:hover and the .status-active/.status-revoked/.status-expired background/border rules) to use the corresponding token-based values (either rgba(var(--color-danger-rgb), <alpha>) / rgba(var(--color-success-rgb), <alpha>) / rgba(var(--color-warning-rgb), <alpha>) if you have RGB tokens, or use transparentized token colors derived from --color-... with appropriate alpha). Ensure you change the selectors mentioned (`#delete-key-modal` .modal-content, `#delete-key-modal` .modal-icon, .btn-danger:hover, .status-active, .status-revoked, .status-expired) so all visual values come from the theme tokens.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@static/css/dashboard/dashboard-base.css`:
- Line 1: Stylelint fails on the `@import` url(...) notation; change the import to
the string form that Stylelint expects by replacing the `@import` rule in
static/css/dashboard/dashboard-base.css (the current "@import
url('https://fonts.googleapis.com/css2?family=Allerta+Stencil&display=swap');")
with the quoted string form so the stylesheet linter accepts it.
- Around line 218-227: The CSS currently hides the sidebar toggle visually while
leaving it focusable; update the rules for the selectors .collapsed
.sidebar-toggle and .collapsed .sidebar-header:hover .sidebar-toggle so that the
toggle also becomes visible when it receives keyboard focus or when its header
becomes focused (use :focus and/or :focus-within on .sidebar-toggle and
.sidebar-header) in addition to :hover; keep the existing opacity transition and
flex/margin rules so the control fades in the same way for both mouse and
keyboard interactions.
In `@static/css/header.css`:
- Around line 356-366: Remove the dead duplicate text-align declaration in the
.self-promo-inner a rule: the first "text-align: center" that appears before
"all: unset" should be deleted because all: unset negates any prior
declarations, leaving only the later "text-align: center" (after font-family) to
take effect; update the CSS by removing the redundant property so the selector
.self-promo-inner a no longer contains two text-align declarations.
In `@static/js/dashboard/dashboard-base.js`:
- Around line 1-9: In authFetch the caller-provided options.headers overwrites
the merged default headers because you spread options after defaultOptions; fix
by explicitly merging headers when building the final fetch options (e.g.,
compute mergedHeaders = { ...defaultOptions.headers, ...options.headers } and
then call fetch with { ...defaultOptions, ...options, headers: mergedHeaders })
so that credentials and Content-Type from defaultOptions remain while allowing
callers to override or add header keys; update the authFetch function to use
this mergedHeaders approach.
In `@static/js/dashboard/links.js`:
- Around line 284-304: setupQRCodeInteractions is adding duplicate event
listeners each time it's called; to fix, use named handler functions and remove
existing listeners before adding or attach listeners only once. Specifically,
define named functions for click (e.g., onQrClick) and the mouseenter/mouseleave
handlers, call qrContainer.removeEventListener for each named handler before
calling qrContainer.addEventListener, and keep references to the handlers so
repeated calls to setupQRCodeInteractions won't stack duplicates; use the
existing element IDs (qrContainer, qrOverlay) and the download function name
(downloadQRCodeFromModal) to wire the click handler.
- Around line 338-348: In validateUrl, stop using
normalized.toLowerCase().includes('spoo.me') and instead parse the normalized
string into a URL object (e.g., new URL(normalized) inside a try/catch) and
inspect its hostname; reject only when hostname === 'spoo.me' or
hostname.endsWith('.spoo.me') (case-insensitive). Keep existing URL validation
first, handle parse errors gracefully, and use the hostname check to block
self-shortening rather than substring matching on the whole URL.
In `@templates/dashboard/statistics.html`:
- Around line 209-216: The icon-only toggle buttons lack accessible names;
update each element with class "view-toggle-btn" (both data-view="chart" and
data-view="table") inside the "view-toggle" group to include an explicit
aria-label like "Chart view" and "Table view" respectively, and apply the same
fix to icon-only cascade triggers using class "cascade-btn" (or those with
data-action/cascade attributes) so each has a clear aria-label describing its
action; keep existing aria-pressed/title attributes but add the aria-label to
ensure screen readers announce the button purpose.
---
Outside diff comments:
In `@templates/base.html`:
- Around line 14-43: Update templates/legal/base.html to align cache-busting
with the main template by changing the header asset versions: bump the header
CSS query param from ?v=6 to ?v=7 and the header JS query param from ?v=6 to
?v=7 so the file references match templates/base.html (this ensures the
self-promo styles in header.css and the close-button wiring in header.js are
loaded for legal pages).
---
Duplicate comments:
In `@static/js/dashboard/links.js`:
- Around line 717-720: The helper toEpochSeconds currently uses new Date(value)
which doesn't throw on invalid input and results in NaN that serializes as null;
update toEpochSeconds to explicitly validate the created Date (e.g., const d =
new Date(value); const t = d.getTime(); if (!value || Number.isNaN(t)) return
undefined;) and otherwise return Math.floor(t/1000) so invalid/empty inputs
produce undefined instead of NaN/null in the filter JSON.
- Around line 99-111: The QR image loading block only handles successful load
via qrImg.onload, so failures leave the loading animation and invisible image;
add an error handler (qrImg.onerror) that clears the parent animation, restores
visibility (e.g., set qrImg.style.opacity = "1" or show a fallback), and calls
or skips setupQRCodeInteractions as appropriate; ensure the same cleanup done in
qrImg.onload (clearing parentElement.style.animation and adjusting opacity) is
performed in qrImg.onerror for the element referenced by qrImg and avoid leaving
dangling event handlers.
- Around line 308-310: The guard in downloadQRCodeFromModal incorrectly negates
naturalWidth; update it to return early when the element is missing or the image
hasn't loaded by checking the naturalWidth value directly (e.g., use a condition
like "if (!qrImg || qrImg.naturalWidth === 0) return" or equivalently "if
(!qrImg || !qrImg.naturalWidth) return"); this fixes the unloaded-image check
for the element with id "created-link-qr" inside the downloadQRCodeFromModal
function.
In `@static/js/dropdown.js`:
- Around line 156-168: The init() function currently applies menu semantics to
every .dropdown; change it so you only set aria-haspopup="menu" on the trigger
(via getTrigger(d)) and role="menu" on the menu (via getMenu(d)) when the panel
is a true action menu: detect this by checking for a marker (e.g., a data
attribute like data-dropdown-type="menu" or an existing role) or by verifying
the menu does NOT contain form controls (inputs, selects, textareas, buttons)
before setting semantics; leave panels containing form controls untouched so
they retain native form semantics. Ensure to update both the
trigger.setAttribute('aria-haspopup','menu') /
trigger.setAttribute('aria-expanded','false') and the
menu.setAttribute('role','menu') calls to be conditional based on that check.
---
Nitpick comments:
In `@static/css/dashboard/keys.css`:
- Around line 1155-1305: Duplicate modal styling exists for `#delete-key-modal`
and the links url-modal; extract shared styles into a reusable class (e.g.,
.confirm-modal or .danger-confirm-modal) in a common stylesheet and have
`#delete-key-modal` simply add that class. Move shared selectors like
.modal-container, .modal-content, .modal-header, .modal-body, .modal-footer,
.modal-title-section-delete, .modal-icon, .modal-title, .btn-cancel and
.btn-delete (including transitions, backdrop, sizing, padding, borders, and
responsive rules) into the new shared class, keep only ID-specific overrides
(colors or wording) in the existing `#delete-key-modal` block, and update the
other modal (url-modal) to use the same shared class so JS can still target the
element by ID while styles are centralized.
- Around line 1186-1230: Replace hard-coded radius and color literals with the
theme tokens: in `#delete-key-modal` .modal-content swap border-radius: 16px for
border-radius: var(--radius-lg); in `#delete-key-modal` .modal-icon replace
linear-gradient(135deg, `#ef4444`, `#dc2626`) with linear-gradient(135deg,
var(--color-danger), var(--color-danger-hover)) (or the hover token your theme
uses). Also update other literal-danger/success/warning rgba usages referenced
in the comment (.btn-danger:hover and the
.status-active/.status-revoked/.status-expired background/border rules) to use
the corresponding token-based values (either rgba(var(--color-danger-rgb),
<alpha>) / rgba(var(--color-success-rgb), <alpha>) /
rgba(var(--color-warning-rgb), <alpha>) if you have RGB tokens, or use
transparentized token colors derived from --color-... with appropriate alpha).
Ensure you change the selectors mentioned (`#delete-key-modal` .modal-content,
`#delete-key-modal` .modal-icon, .btn-danger:hover, .status-active,
.status-revoked, .status-expired) so all visual values come from the theme
tokens.
In `@static/js/header.js`:
- Around line 139-149: Move the self-promo wiring into the existing
DOMContentLoaded handler: remove the top-level query/select and event hookup for
closeButton and instead perform document.querySelector('.self-promo-close') and
addEventListener('click', closeSelfPromo) inside the existing DOMContentLoaded
callback (so the binding runs with the other DOM initializations); you can keep
the closeSelfPromo function where it is or move its declaration into the same
callback for locality, but ensure closeSelfPromo is defined before it's
referenced.
In `@templates/dashboard/settings.html`:
- Line 6: The template currently appends manual cache-busters like "?v=1" to
assets (e.g., the stylesheet link for "css/dashboard/settings.css" and the
scripts referenced around lines 239–241); change this to a centralized solution
by either using Jinja's url_for query argument (e.g., url_for('static',
path='css/...', v=app.config['ASSET_VERSION'])) or replace the manual suffix
with a helper that returns a file-hash or global version variable (config key
like ASSET_VERSION or a template helper function get_asset_url) and use that for
all asset references so cache-busting is automatic and consistent across
settings.css and the referenced script tags.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e6fba49c-7222-419d-8573-a023e1d0a4e0
📒 Files selected for processing (30)
static/css/confetti.cssstatic/css/dashboard/dashboard-base.cssstatic/css/dashboard/keys.cssstatic/css/dashboard/links.cssstatic/css/dashboard/url-modal.cssstatic/css/header.cssstatic/css/result.cssstatic/css/self-promo.cssstatic/js/dashboard/dashboard-base.jsstatic/js/dashboard/links.jsstatic/js/dropdown.jsstatic/js/field-error.jsstatic/js/header.jsstatic/js/modal-tabs.jsstatic/js/notifications.jsstatic/js/self-promo.jsstatic/js/sheet-handle.jsstatic/js/stats-script.jsstatic/js/verification-check.jstemplates/api.htmltemplates/base.htmltemplates/dashboard/base.htmltemplates/dashboard/keys.htmltemplates/dashboard/links.htmltemplates/dashboard/settings.htmltemplates/dashboard/statistics.htmltemplates/index.htmltemplates/result.htmltemplates/stats.htmltests/smoke/test_routes_registered.py
💤 Files with no reviewable changes (5)
- static/js/stats-script.js
- tests/smoke/test_routes_registered.py
- static/css/self-promo.css
- static/js/self-promo.js
- static/css/confetti.css
✅ Files skipped from review due to trivial changes (3)
- static/js/verification-check.js
- static/js/sheet-handle.js
- static/js/field-error.js
🚧 Files skipped from review as they are similar to previous changes (6)
- templates/stats.html
- templates/dashboard/keys.html
- templates/dashboard/base.html
- static/js/notifications.js
- static/js/modal-tabs.js
- static/css/dashboard/links.css
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (3)
templates/dashboard/statistics.html (1)
209-218:⚠️ Potential issue | 🟡 MinorIcon-only chart controls still need explicit accessible names.
The previous accessibility finding still applies:
view-toggle-btnandcascade-btnbuttons have only icons plustitle. Addaria-labelto each button, and whenstatic/js/dashboard/statistics.jsupdates cascade trigger titles, mirror that value toaria-labeltoo.♿ Example fix for one block
- <button class="view-toggle-btn active" data-view="chart" aria-pressed="true" title="Chart view"> + <button type="button" class="view-toggle-btn active" data-view="chart" aria-pressed="true" aria-label="Chart view" title="Chart view"> @@ - <button class="view-toggle-btn" data-view="table" aria-pressed="false" title="Table view"> + <button type="button" class="view-toggle-btn" data-view="table" aria-pressed="false" aria-label="Table view" title="Table view"> @@ - <button type="button" class="cascade-btn dropdown-trigger" data-dropdown-trigger data-value="compare" title="Compare Both"> + <button type="button" class="cascade-btn dropdown-trigger" data-dropdown-trigger data-value="compare" title="Compare Both" aria-label="Compare Both">Also applies to: 297-306, 351-360, 408-417, 461-470, 518-527, 572-581
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/dashboard/statistics.html` around lines 209 - 218, The icon-only buttons (elements with class "view-toggle-btn" and the cascade button "cascade-btn" / attribute data-dropdown-trigger) lack accessible names; add an aria-label to each button (e.g., for the chart/table toggle buttons and the cascade dropdown triggers tied to data-chart="timeSeriesChart") that matches the button's purpose, and update static/js/dashboard/statistics.js so whenever it changes a button's title (e.g., on the cascade trigger or toggles) it also sets the corresponding aria-label to the same value (use the same selectors: .view-toggle-btn, .cascade-btn, [data-dropdown-trigger], and data-chart attributes). Ensure all other similar blocks (the other line ranges mentioned) get the same aria-label treatment.static/js/dashboard/links.js (2)
1004-1024:⚠️ Potential issue | 🟠 MajorDuplicate
initSegments()still present — segments get initialized twice.This second
initSegments()(lines 1005-1020) is a reduced copy of the one defined at lines 3-28 — it does not toggle the.activeclass on buttons, while the first one does. Both are invoked (line 30 and line 1024), so click handlers are registered twice per button and the second init silently clobbers the active-class work the first one did on hydration.♻️ Proposed fix — remove the duplicate and reuse the full version
- // segmented controls behavior - function initSegments() { - const segs = document.querySelectorAll('.seg'); - segs.forEach(seg => { - const targetId = seg.getAttribute('data-target'); - const hidden = document.getElementById(targetId); - const buttons = Array.from(seg.querySelectorAll('button[data-value]')); - const indexByValue = new Map(buttons.map((b, i) => [b.getAttribute('data-value'), i])); - function apply(value) { - if (hidden) { hidden.value = value; } - const idx = indexByValue.has(value) ? indexByValue.get(value) : 0; - seg.setAttribute('data-active', String(idx)); - } - apply(hidden ? hidden.value : ''); - buttons.forEach(btn => btn.addEventListener('click', () => apply(btn.getAttribute('data-value') || ''))); - }); - } - // Restore saved filter state before segmented visuals hydrate. loadFilters(); - initSegments(); + // The top-level DOMContentLoaded handler already calls initSegments() (line 30); + // just nudge each segment's hidden input to re-sync after loadFilters() ran. + document.querySelectorAll('.seg').forEach(seg => { + const hidden = document.getElementById(seg.getAttribute('data-target')); + if (hidden) hidden.dispatchEvent(new Event('input', { bubbles: true })); + });If you prefer to keep a local re-hydration step here (since
loadFilters()mutates hidden inputs after the firstinitSegments()runs), hoist the full version from lines 3-28 into module scope and call it once from each site instead of redefining it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/js/dashboard/links.js` around lines 1004 - 1024, There is a duplicate, reduced initSegments() definition that re-registers click handlers and overwrites active-class behavior from the original; remove the second (reduced) initSegments implementation and reuse the full module-scoped initSegments function (or hoist the full implementation into module scope) so only one initSegments runs; ensure callers still invoke loadFilters() before initSegments() if needed so hidden inputs are restored, and keep the original initSegments logic that toggles button .active classes and sets data-active.
314-316:⚠️ Potential issue | 🔴 CriticalCritical: the
naturalWidth === 0guard is still inverted — condition is always false.
!qrImg.naturalWidth === 0parses as(!qrImg.naturalWidth) === 0, which compares a boolean to0and is never true. The early-return never fires, sodrawImagewill be called on an unloaded image and produce a blank/broken PNG download.🐛 Proposed fix
- if (!qrImg || !qrImg.naturalWidth === 0) return; + if (!qrImg || qrImg.naturalWidth === 0) return;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@static/js/dashboard/links.js` around lines 314 - 316, In downloadQRCodeFromModal, the guard incorrectly uses "!qrImg.naturalWidth === 0" which always evaluates false; update the early-return to check either the element absence or that its naturalWidth equals zero (i.e., return early if qrImg is falsy OR qrImg.naturalWidth === 0) so drawImage is only called on a fully loaded image; modify the condition around the element with id "created-link-qr" accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@middleware/security.py`:
- Around line 129-142: The middleware StaticCacheHeadersMiddleware currently
marks every /static/* response as "public, max-age=31536000, immutable"; change
dispatch to only apply the year-long immutable header to assets that are clearly
versioned (e.g., contain a cache-busting query like "?v=" or a hashed filename
pattern such as .{8,}.[ext] or a manifest-based lookup), and for other /static/
responses set a shorter, non-immutable Cache-Control (e.g., public,
max-age=3600) so unversioned files (like /static/images/favicon.png referenced
in templates/api.html) can be refreshed; update the header logic in dispatch
(use request.url.path and request.url.query or pattern match on the final path)
accordingly.
In `@static/js/dashboard/dashboard-base.js`:
- Around line 1-10: The authFetch implementation allows callers to override
credentials because restOptions is spread after the default credentials; update
authFetch so credentials cannot be overridden by either removing credentials
from restOptions during destructuring (e.g., const { headers: callerHeaders,
credentials, ...restOptions } = options) or by applying ...restOptions before
setting credentials, and then always set credentials: 'include' last in the
fetch options; refer to the authFetch function and the restOptions/callerHeaders
identifiers when making the change.
In `@templates/contact.html`:
- Around line 31-34: The inline calls to showNotification({{ error | tojson }},
'error', 10000) and showNotification({{ success | tojson }}, 'success', 10000)
execute before notifications.js is loaded (notifications.js is deferred in
templates/base.html), causing ReferenceError; move these calls into the
templates' scripts block (the {% block scripts %} section) or wrap them in a
DOM-ready/DOMContentLoaded check that waits for showNotification to exist before
invoking it (refer to showNotification and notifications.js) so notifications
run only after the deferred script has loaded.
In `@templates/dashboard/links.html`:
- Around line 272-276: The span with id "create-alias-status" is currently
aria-hidden and so screen readers miss alias availability updates; change that
element to be accessible by removing aria-hidden and adding a polite live region
(e.g., role="status" and aria-live="polite" and aria-atomic="true") so dynamic
messages are announced, and apply the same change to the other status span
instance (the one around lines 462-466) so both alias checkers expose feedback
to assistive tech; keep the existing IDs ("create-alias-status") so the JS that
updates the element continues to work.
In `@templates/dashboard/statistics.html`:
- Around line 156-158: The export icon button (the element with classes
"export-btn" and "dropdown-trigger") lacks an explicit accessible name; add an
aria-label="Export" attribute to that button element so assistive technologies
get a stable, descriptive name in addition to the existing title attribute.
- Around line 160-173: The dropdown primitive expects selected value in
data-value but the export buttons only set data-format; update each export
button element in the statistics template (the buttons that currently have
data-format="json|csv|xlsx|xml") to also include matching data-value attributes
(e.g., data-value="json", etc.) so the dropdown:select event dispatched by the
dropdown code will carry the standardized detail.value and the export handler in
statistics.js (the handler that reads e.detail.value ||
e.detail.item.getAttribute('data-format')) will receive the value via the
primary path.
---
Duplicate comments:
In `@static/js/dashboard/links.js`:
- Around line 1004-1024: There is a duplicate, reduced initSegments() definition
that re-registers click handlers and overwrites active-class behavior from the
original; remove the second (reduced) initSegments implementation and reuse the
full module-scoped initSegments function (or hoist the full implementation into
module scope) so only one initSegments runs; ensure callers still invoke
loadFilters() before initSegments() if needed so hidden inputs are restored, and
keep the original initSegments logic that toggles button .active classes and
sets data-active.
- Around line 314-316: In downloadQRCodeFromModal, the guard incorrectly uses
"!qrImg.naturalWidth === 0" which always evaluates false; update the
early-return to check either the element absence or that its naturalWidth equals
zero (i.e., return early if qrImg is falsy OR qrImg.naturalWidth === 0) so
drawImage is only called on a fully loaded image; modify the condition around
the element with id "created-link-qr" accordingly.
In `@templates/dashboard/statistics.html`:
- Around line 209-218: The icon-only buttons (elements with class
"view-toggle-btn" and the cascade button "cascade-btn" / attribute
data-dropdown-trigger) lack accessible names; add an aria-label to each button
(e.g., for the chart/table toggle buttons and the cascade dropdown triggers tied
to data-chart="timeSeriesChart") that matches the button's purpose, and update
static/js/dashboard/statistics.js so whenever it changes a button's title (e.g.,
on the cascade trigger or toggles) it also sets the corresponding aria-label to
the same value (use the same selectors: .view-toggle-btn, .cascade-btn,
[data-dropdown-trigger], and data-chart attributes). Ensure all other similar
blocks (the other line ranges mentioned) get the same aria-label treatment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2a34beb0-33da-438b-b10e-0dfbc09d92f3
📒 Files selected for processing (24)
app.pymiddleware/security.pystatic/css/dashboard/dashboard-base.cssstatic/css/dashboard/settings.cssstatic/css/header.cssstatic/css/mobile-header.cssstatic/js/dashboard/dashboard-base.jsstatic/js/dashboard/links.jstemplates/api.htmltemplates/base.htmltemplates/contact.htmltemplates/dashboard/apps.htmltemplates/dashboard/base.htmltemplates/dashboard/keys.htmltemplates/dashboard/links.htmltemplates/dashboard/settings.htmltemplates/dashboard/statistics.htmltemplates/index.htmltemplates/legal/base.htmltemplates/preview.htmltemplates/report.htmltemplates/stats.htmltemplates/stats_view.htmltemplates/verify.html
✅ Files skipped from review due to trivial changes (5)
- templates/preview.html
- templates/verify.html
- templates/stats_view.html
- templates/dashboard/keys.html
- static/css/dashboard/settings.css
🚧 Files skipped from review as they are similar to previous changes (8)
- templates/report.html
- templates/index.html
- templates/dashboard/settings.html
- templates/base.html
- templates/dashboard/apps.html
- templates/legal/base.html
- templates/dashboard/base.html
- templates/stats.html
…roved maintainability
…ed maintainability
…istency; remove redundant notification calls in link success modal Refactor CSS styles for consistency and readability - Standardized spacing and formatting in various CSS files, including contacts-modal.css, billing.css, settings.css, statistics.css, and others. - Improved readability by adding spaces after commas and around operators. - Ensured consistent use of newlines at the end of files. - Updated HTML templates to enhance readability and maintainability, including adjustments in base.html, billing.html, links.html, and statistics.html. - Enhanced the verification banner styles for better responsiveness and visual appeal.
- Introduced a new JavaScript module for managing field errors, allowing for dynamic error display and styling. - Updated CSS for input groups to improve focus styles and error indications. - Refactored alias validation logic to provide clearer error messages and enforce character limits. - Adjusted template links to include versioning for CSS and JS files, ensuring proper cache management.
…nality - Added a new dropdown CSS file for styling dropdown components, including trigger and menu states. - Introduced a JavaScript module for dropdown functionality, supporting open, close, and toggle actions, as well as keyboard navigation. - Updated the dashboard statistics template to utilize the new dropdown structure, enhancing the auto-refresh feature. - Refactored existing auto-refresh dropdown handling to leverage the new dropdown primitive for improved maintainability.
- Introduced a new JavaScript module `modal-tabs.js` to manage tab functionality within modals, including auto-initialization and active tab management. - Refactored existing tab handling in `url-manager.js` and `links.js` to utilize the new `ModalTabs` module, simplifying the code and enhancing maintainability. - Updated templates to include the new modal tabs script, ensuring proper integration across the application.
- Added visual error indicators to tabs in modals, improving user feedback on form validation. - Implemented functions to refresh error states and jump to the first invalid tab, streamlining error management. - Updated JavaScript modules to integrate new error handling features, ensuring a cohesive user experience across modals. - Adjusted templates to reflect changes in error handling and ensure proper functionality. feat: enhance dashboard sidebar and confetti effects - Updated sidebar styles to support a responsive logo display, showing a full logo when expanded and a square favicon when collapsed. - Improved confetti effect by reducing particle count and adjusting animation timing for a more subtle celebration experience. - Refactored confetti triggering logic to ensure compatibility with reduced motion preferences, enhancing accessibility. - Updated versioning for CSS and JS files in templates to reflect recent changes.
- Added functionality to save and load filter states for links in local storage, enhancing user experience by retaining user selections. - Implemented time range persistence for the statistics dashboard, allowing users to maintain their selected date ranges across sessions. - Updated JavaScript files to include new save and load methods for filters and time ranges, ensuring data consistency. - Incremented version numbers for affected JavaScript files in templates to reflect recent changes.
- Implemented a new modal for confirming the deletion of API keys, enhancing user experience by providing a clear warning before irreversible actions. - Updated JavaScript to manage modal visibility and handle delete confirmation, including cancel and confirm button functionalities. - Added corresponding CSS styles for the modal to ensure proper display and responsiveness. - Incremented version numbers for affected CSS and JS files in templates to reflect recent changes.
- Replaced existing table view button styles with a new segmented toggle for chart and table views, enhancing user interaction. - Updated JavaScript to manage the new toggle functionality, allowing users to switch between chart and table views seamlessly. - Adjusted CSS styles for the new toggle buttons to ensure proper display and responsiveness. - Incremented version numbers for affected CSS and JS files in templates to reflect recent changes.
- Introduced a new endpoint for checking the availability of custom aliases, enhancing user experience by providing immediate feedback on alias validity. - Implemented client-side alias validation with real-time feedback, including length and format checks. - Added corresponding UI elements and styles for alias input, including a random alias generator button. - Updated relevant JavaScript to manage alias checking and integrate with the existing URL shortening functionality. - Incremented version numbers for affected CSS and JS files in templates to reflect recent changes. style: update tab styles in URL modal - Enhanced tab styling by adding padding, font size adjustments, and gap spacing for improved layout. - Updated icon font size within tabs for better visual hierarchy. - Incremented version number for the URL modal CSS file to reflect changes.
- Refactored kebab and profile dropdowns to utilize a shared dropdown primitive, enhancing consistency and maintainability. - Updated CSS classes for dropdown menus and items to align with the new structure, improving styling and interaction. - Adjusted JavaScript to manage dropdown functionality more effectively, including event handling for open/close actions. - Incremented version numbers for affected CSS and JS files in templates to reflect recent changes. refactor: unify dropdown handling and update styles for consistency - Refactored dropdown components in links and statistics to utilize a shared primitive for improved usability and maintainability. - Updated CSS classes for dropdowns and buttons to align with the new structure, enhancing styling and interaction. - Adjusted JavaScript to streamline dropdown open/close functionality, ensuring consistent behavior across the dashboard. - Incremented version numbers for affected CSS files in templates to reflect recent changes. feat: implement bottom-sheet handle functionality across dashboard components - Added a new JavaScript module for handling bottom-sheet interactions, allowing users to swipe down or tap to dismiss sheets. - Updated CSS styles for dropdowns and sheets to include a functional handle for improved usability and consistency. - Adjusted existing components in links, statistics, and date range picker to incorporate the new handle, enhancing user experience. - Incremented version numbers for affected CSS and JS files in templates to reflect recent changes. style: remove border-radius from clickable rows and update chart options - Removed border-radius from clickable rows in the URL modal CSS for a cleaner look. - Added maxBarThickness to chart options in the statistics dashboard for improved bar chart display. - Padded empty slots in chart data to ensure proper alignment for charts with fewer categories. - Incremented version number for the statistics JavaScript file in the template to reflect changes. refactor: remove unused test for API v1 route count - Deleted the test for verifying the number of API v1 route paths as it was deemed unnecessary. - Cleaned up the test file to improve maintainability and focus on relevant tests. refactor: remove unused CSS and JavaScript files, update styles for self-promo and dropdown components - Deleted unused CSS files for confetti, dropdown, and self-promo to streamline the codebase. - Moved self-promo styles into header.css for better organization and maintainability. - Updated header.css to include new styles for self-promo and dropdown components, enhancing layout and user interaction. - Incremented version numbers for affected CSS files in templates to reflect changes. style: enhance button and layout styles across dashboard components - Updated button hover styles in keys and links CSS to include brightness, translation, and shadow effects for improved user interaction. - Centered text elements and adjusted margins for better layout consistency in various components. - Implemented flexbox layout adjustments for responsive design in dashboard content and modal sections. - Incremented version numbers for affected CSS and JS files in templates to reflect changes.
…ic assets - Introduced StaticCacheHeadersMiddleware to set long-lived immutable Cache-Control headers on responses for /static/* paths. - Updated app.py to integrate the new middleware, enhancing performance by allowing browsers to cache static assets for a year. style: update z-index values and clean up CSS styles across header and dashboard components - Adjusted z-index values in header and mobile-header CSS files to ensure proper stacking context with fallback values. - Removed unnecessary text alignment from self-promo styles in header CSS for cleaner design. - Enhanced accessibility by adding ARIA attributes to modal components in the dashboard. - Incremented version numbers for affected CSS and JS files in templates to reflect changes.
Zingzy
force-pushed
the
feat/frontend-rearchirecture
branch
from
dd513bc to
c6b7ddf
Compare
…ification scripts for DOM readiness - Restored the inclusion of credentials in the authFetch function to maintain session integrity during API calls. - Updated notification scripts in multiple templates to ensure they execute after the DOM is fully loaded, improving user experience by preventing potential timing issues.
github-project-automation
Bot
moved this from 🏗️ In Progress
to ✔️ Done
in spoo.me Development Roadmap
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
UI/Design
Polish