Feat/shadcn UI restyling#87
Merged
Merged
Conversation
Replace DaisyUI-specific styling with shadcn/ui-inspired design system across all components and LiveViews. Add Geist/Geist Mono fonts, CSS custom properties for light/dark theming, and Tailwind v4 theme extension with semantic color tokens (background, foreground, muted, border, etc.). Neutralize DaisyUI shadow/radius defaults so Tailwind utilities win. Update all ~25 LiveView files to use consistent shadcn-style classes for buttons, inputs, tables, cards, and dropdowns. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… unify table styling - Add filter_bar/1 with JS.toggle popover, search input, and filter chips - Add filter_chip/1 for removable active filter pills - Add pagination/1 with always-visible Previous/Page X of Y/Next footer - Refactor invoice index to use new components with remove_filter and clear_filters event handlers - Wrap all tables app-wide in rounded-lg border container - Standardize button/input height to h-9 across all views - Update team_live raw table to match .table component styling Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add badge, card, auth_card, logo, nav_item_list, file_upload_dropzone components - Extend button with size attr (default/sm/icon) and success/warning variants - Migrate all LiveViews to use component library instead of hardcoded CSS - Deduplicate format_datetime/format_date helpers via shared imports - Fix bugs: "Invoi" typo, conflicting padding, dead handler, DaisyUI remnants - Replace DaisyUI dropdown with JS.toggle, DaisyUI avatar with pure Tailwind - Neutralize DaisyUI shadows in @layer base, remove all shadow-xs/shadow-sm - Use outline variant for table action buttons (Edit/Delete/Remove/Revoke) - Add consistent focus-visible states to all form inputs - Restructure comment layout for proper text alignment Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The home/landing screen was redundant since the login page already has a "Sign up" link. Now `/` redirects to `/users/log-in` directly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds shadcn-style theming tokens and dark-mode support; introduces a set of reusable UI components (badge, card, auth_card, pagination, filter_bar, etc.); refactors layouts and many LiveViews to use those components and tokens; replaces home render with redirect to login; updates tests to match new markup. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant ExportLive as ExportLive (LiveView)
participant ExportsSvc as Exports.create_export
participant DB as Repo/DB
Client->>ExportLive: phx-submit "export" (params with _action="preview" or "export")
ExportLive->>ExportLive: validate params (do_preview / do_export_action)
alt preview
ExportLive->>ExportLive: compute preview_count
ExportLive-->>Client: render/patch preview_count
else export
ExportLive->>ExportsSvc: Exports.create_export(build_params)
ExportsSvc->>DB: insert export batch
DB-->>ExportsSvc: ack
ExportsSvc-->>ExportLive: {:ok, export}
ExportLive->>ExportLive: stream_new_batch
ExportLive-->>Client: render updated exports list / flash
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
lib/ksef_hub_web/components/layouts.ex (1)
66-80:⚠️ Potential issue | 🟠 MajorDon't post the old company-scoped path as
return_to.When the current page is something like
/c/1/exports, selecting company 2 still submitsreturn_to=/c/1/exports. If the switch-company action redirects to that value, the user lands back on the previous company's route and the selector looks broken. Generate a target-company path here, or rewrite the/c/:id/...prefix before submitting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ksef_hub_web/components/layouts.ex` around lines 66 - 80, The hidden return_to currently uses `@current_path` which can point to another company's /c/:id/... URL causing a redirect back to the wrong company; update the form in the company loop (the :for over `@companies`, company.id and the hidden "return_to") to produce a target-company path instead of blindly reusing `@current_path` — either rewrite the leading /c/:id/ segment of `@current_path` to use the current company.id (when `@current_path` matches ^/c/\d+/) or build a safe fallback path for the company (e.g. invoices path) and set that as the hidden return_to value so the switch-company action always redirects to the same company selected in the form.test/ksef_hub_web/live/invoice_live/show_test.exs (1)
186-190:⚠️ Potential issue | 🟠 MajorThis assertion doesn't prove the approve guard held.
Income invoices don't show an approved badge here, so this can stay green even if the event accidentally mutates the record. Assert the persisted status is still unchanged after the event, and optionally the error flash too.
💡 Example
# Buttons aren't shown for income, but test the server-side guard via hook render_hook(view, "approve", %{}) - # Status badge is hidden for income invoices, and status should not change to approved - refute has_element?(view, "[class*=bg-success]", "approved") + assert Invoices.get_invoice!(company.id, invoice.id).status == :pending🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/ksef_hub_web/live/invoice_live/show_test.exs` around lines 186 - 190, The current test uses render_hook(view, "approve", %{}) and then only checks the absence of a success badge via has_element?, which doesn't guarantee the invoice wasn't mutated; after calling render_hook in the test (invoice_live/show_test.exs) fetch the invoice record from the database (e.g., via Repo.get!/Repo.reload or the existing context function used elsewhere) and assert its status field remains the original (unchanged) value, and optionally assert the live view flash contains an error message if expected; keep the render_hook(view, "approve", %{}) call and add the DB fetch + assert on the persisted invoice status (and flash check) to prove the server-side guard held.lib/ksef_hub_web/live/invoice_live/index.ex (1)
44-80:⚠️ Potential issue | 🟡 MinorNormalize restricted
typeparams before rebuilding the filter UI.For roles that cannot view all invoice types, the effective query can still return expense rows while
filter_assigns/4reflects the rawtype=incomeparam in the form/chips. That leaves the page showing expense results with an activeIncomefilter in the UI. Strip or coerce unauthorized:typefilters before passing them intofilter_assigns/4.💡 Example
def handle_params(params, _uri, socket) do - filters = parse_filters(params) - role = socket.assigns[:current_role] + filters = parse_filters(params) + + filters = + if Authorization.can?(role, :view_all_invoice_types), + do: filters, + else: Map.delete(filters, :type) result = case socket.assigns[:current_company] do %{id: company_id} -> Invoices.list_invoices_paginated(company_id, filters, role: role)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ksef_hub_web/live/invoice_live/index.ex` around lines 44 - 80, filter_assigns/4 currently uses the raw filters to build the form and active_filters which can show an unauthorized "type" chip; normalize the filters[:type] at the top of filter_assigns by checking Authorization.can?(role, :view_all_invoice_types) and, if the role cannot view all types, clear or coerce filters[:type] to nil/"" (or an allowed default) before you build the form (the "type" key passed into to_form) and before calling build_active_filters so the UI no longer displays an unauthorized type filter; update only the local filters variable used by the form and build_active_filters (leave the original query semantics intact elsewhere).
🧹 Nitpick comments (7)
lib/ksef_hub_web/live/category_live.ex (1)
171-274: Consider splitting the form and table into local function components.
render/1now mixes LiveView state wiring with two large UI blocks. Pulling these into stateless components likecategory_form/1andcategory_table/1would make future restyling easier to review and less error-prone.Based on learnings "Applies to **/_live.ex : Extract reusable logic into function components" and "Applies to **/_live.ex : Use stateless function components when possible".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ksef_hub_web/live/category_live.ex` around lines 171 - 274, The render/1 is too large and mixes LiveView state with UI; extract the form and table into two stateless function components category_form/1 and category_table/1, moving the form markup (inputs using `@form`, `@editing`, translate_error, phx-submit/validate/cancel_edit) into category_form(assigns) and the table markup (rows=@streams.categories, row_id/row_item, action buttons with phx-click edit/delete and data-confirm) into category_table(assigns); then simplify render/1 to call ~H{ category_form(`@assigns`) category_table(`@assigns`) } and ensure both components accept and use the same assigns they reference (form, editing, streams, etc.) so event bindings and helpers continue to work.lib/ksef_hub_web/controllers/page_html/home.html.heex (2)
25-36: Add unique DOM IDs to action buttons for testability.The coding guidelines recommend adding unique DOM IDs to key elements for testing purposes. The "Log in" and "Create an account" buttons are missing IDs (unlike the "Go to Companies" button which has
id="primary-cta-companies").As per coding guidelines: "Always add unique DOM IDs to key elements (like forms, buttons, etc) when writing templates, these IDs can later be used in tests."
♻️ Proposed fix
<a href={~p"/users/log-in"} class="inline-flex items-center justify-center gap-2 h-9 px-4 text-sm font-medium rounded-md bg-shad-primary text-shad-primary-foreground hover:bg-shad-primary/90 transition-colors cursor-pointer w-full mb-3" + id="home-login-button" > Log in </a> <a href={~p"/users/register"} class="inline-flex items-center justify-center gap-2 h-9 px-4 text-sm font-medium rounded-md border border-input bg-background hover:bg-shad-accent hover:text-shad-accent-foreground transition-colors cursor-pointer w-full" + id="home-register-button" > Create an account </a>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ksef_hub_web/controllers/page_html/home.html.heex` around lines 25 - 36, Add unique DOM id attributes to the two action anchors for testability: the link with href ~p"/users/log-in" (text "Log in") should get a stable id such as "primary-cta-login" and the link with href ~p"/users/register" (text "Create an account") should get a stable id such as "primary-cta-register"; update the <a> elements in the home.html.heex template that render those two buttons to include these id attributes so tests can reliably select them (follow the existing pattern used for id="primary-cta-companies").
5-5: Consider using the.iconcomponent for consistency.The coding guidelines specify to always use the
<.icon>component for icons. While thehero-document-textclass works, using the component ensures consistency with the rest of the codebase.As per coding guidelines: "Always use the
<.icon>component for icons from core_components.ex, never useHeroiconsmodules or similar."♻️ Proposed change
- <span class="hero-document-text size-8 text-primary"></span> + <.icon name="hero-document-text" class="size-8 text-primary" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ksef_hub_web/controllers/page_html/home.html.heex` at line 5, Replace the raw span element that uses the hero-document-text CSS class with the standard icon component: locate the <span class="hero-document-text size-8 text-primary"> and swap it out to call the .icon component from core_components (use the appropriate icon name mapping for the "hero-document-text" glyph and pass the existing size-8 and text-primary classes through the component's class prop) so the template uses .icon consistently across the codebase.lib/ksef_hub_web/components/layouts/root.html.heex (1)
10-15: Consider self-hosting fonts for reliability and privacy.Loading fonts from Google Fonts introduces an external dependency that could affect page load if the CDN is slow/unavailable, and sends user IP addresses to Google. For production applications, consider self-hosting the Geist fonts or using a font proxy.
The implementation itself is correct with proper
preconnecthints anddisplay=swapfor good loading behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ksef_hub_web/components/layouts/root.html.heex` around lines 10 - 15, The page currently loads Google Fonts via external links/hints (the <link> tags that reference https://fonts.googleapis.com and https://fonts.gstatic.com and the href with family=Geist and Geist+Mono); replace these external references by self-hosting the Geist and Geist Mono font files: download the needed font weights, add `@font-face` declarations in your app stylesheet referencing local asset paths, update the HTML to point to the local CSS/font files instead of the Google URLs, and remove the preconnect/link tags to fonts.googleapis.com/fonts.gstatic.com so the app no longer depends on the external CDN.assets/css/app.css (1)
201-240: Minor: Consider unquoting generic font family names.Stylelint flags the quoted
'Geist'font name. While this works, the convention is to quote font names only when they contain spaces or special characters.Suggested fix
body { - font-family: 'Geist', ui-sans-serif, system-ui, -apple-system, sans-serif; + font-family: Geist, ui-sans-serif, system-ui, -apple-system, sans-serif; -webkit-font-smoothing: antialiased; -moz-osx-font-smoothing: grayscale; } code, pre, kbd, .font-mono { - font-family: 'Geist Mono', ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, monospace; + font-family: "Geist Mono", ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, monospace; }Note:
'Geist Mono'needs quotes due to the space, but should use double quotes per CSS conventions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assets/css/app.css` around lines 201 - 240, Unquote the generic family name in the body font stack and standardize quotes for the mono font: in the body rule (selector "body") remove quotes around Geist so it reads Geist, and in the code/pre/kbd/.font-mono rule replace the single-quoted 'Geist Mono' with double quotes "Geist Mono" (keep the rest of the fallback families unchanged) so Stylelint no longer flags the generic name and the multi-word font uses proper double quotes.lib/ksef_hub_web/components/core_components.ex (1)
320-324: Potential false positive in nav_active? path matching.
String.starts_with?(current, path <> "/")could match unintended paths. For example, ifpathis/inv, it would match/invoices/.... Consider requiring an exact match or a trailing slash in the path itself.Suggested fix
defp nav_active?(current, path), - do: current == path || String.starts_with?(current, path <> "/") + do: current == path || String.starts_with?(current, path <> "/") || current == path <> "/"However, if all navigation paths are well-defined (e.g.,
/invoices,/settings), this may not be an issue in practice.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ksef_hub_web/components/core_components.ex` around lines 320 - 324, The nav_active? function may produce unintended matches for similarly prefixed paths; replace the current starts_with? check with a boundary-aware match using Regex: update defp nav_active?(current, path) to return true when current == path or when Regex.match?(~r/^#{Regex.escape(path)}(\/|$)/, current) so only exact path or path followed by a single slash/segment counts as active.test/ksef_hub_web/live/team_live_test.exs (1)
104-104: Scope this assertion to the pending invitations stream.
[data-role='reviewer']now matches anywhere in the table, so a future reviewer member row would make this pass even if the pending invitation row regresses. Anchoring it to#pending-invitations-listkeeps the test focused on the behavior under test.As per coding guidelines, "Always reference the key element IDs you added in the LiveView templates in your tests for `Phoenix.LiveViewTest` functions like `element/2`, `has_element/2`, selectors, etc".🎯 Narrower selector
- assert has_element?(view, "[data-testid='team-table'] [data-role='reviewer']") + assert has_element?(view, "#pending-invitations-list [data-role='reviewer']")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/ksef_hub_web/live/team_live_test.exs` at line 104, The assertion in test/ksef_hub_web/live/team_live_test.exs uses has_element?(view, "[data-testid='team-table'] [data-role='reviewer']") which is too broad; update the selector used in the has_element? call to scope it to the pending invitations stream (anchor to the pending-invitations-list element ID you added) so it targets only the pending invitation row (e.g., prepend the "#pending-invitations-list" id to the selector) and leave the rest of the test logic unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/ksef_hub_web/components/core_components.ex`:
- Around line 217-228: The file fails formatting; run mix format to fix it and
ensure the button/1 function's assigns pattern and the variants map are properly
formatted; locate the button function (def button(%{rest: rest} = assigns) and
the variants map keys like "primary", "outline", "ghost", "destructive",
"success", "warning", nil and reformat so the map and surrounding code comply
with mix format (you can run mix format or apply the formatter to this module to
resolve the CI pipeline failure).
In `@lib/ksef_hub_web/live/category_live.ex`:
- Around line 182-223: Labels are not associated with their inputs; update each
label/input pair to include matching id and for attributes using the form field
ids (e.g., set input id={`@form`[:emoji].id} and label for={`@form`[:emoji].id}),
and do the same for `@form`[:name], `@form`[:description], and `@form`[:sort_order} so
the label elements (Emoji, Name, Description, Order) are programmatically
attached to their corresponding inputs.
- Around line 245-252: Blank emoji/description end up as empty strings (truthy)
so the template's {cat.emoji || "-"} and {cat.description || "-"} render blank;
update atomize_params/1 to convert empty-string values for the emoji and
description keys to nil (or trim and set to nil when result == "") before
building the changeset so they persist as nil in the DB and the template
fallback will work; reference atomize_params/1 and the emoji/description fields
in the changeset logic to implement this normalization.
In `@lib/ksef_hub_web/live/certificate_live.ex`:
- Around line 488-494: The expiry styling in cert_expiry_class doesn't match the
dashboard threshold; update the function cert_expiry_class so certificates with
fewer than 7 days left use the destructive styling (matching dashboard_live.ex's
<7 threshold) and only switch to warning for 7–29 days, keeping the expiry (<0)
case covered by the same destructive rule; modify the cond in cert_expiry_class
accordingly to use the 7-day threshold so both views are consistent.
In `@lib/ksef_hub_web/live/dashboard_live.ex`:
- Around line 144-147: The certificate badge currently only checks `@cert_active`
which can show Active even if the certificate is expired; update the badge logic
in dashboard_live.ex to consider `@cert_expires_at` as well (e.g., treat as Active
only when `@cert_active` is true AND `@cert_expires_at` is in the future), add an
explicit expired branch (render a badge like variant="warning" or "error" when
`@cert_expires_at` is present and in the past) and keep the existing Not
configured branch when `@cert_active` is false and no expiry exists so the UI
never shows Active for expired certs.
In `@lib/ksef_hub_web/live/export_live/index.ex`:
- Around line 167-236: The form currently uses phx-click="preview" and a submit
that ignores form params, causing stale filters; change both actions to submit
the current form payload by making the Preview button submit the form (remove
phx-click="preview") and give both submit buttons a name/value like
name="_action" value="preview" and name="_action" value="export" (keep the
Export button as type="submit" but set name/_action too). Then update the
LiveView submit handler (the event currently handling "export"/submit) to accept
the form params map and branch on params["_action"] => "preview" | "export" to
produce the preview_count or run the export using the submitted
date_from/date_to/invoice_type/only_new values.
In `@lib/ksef_hub_web/live/invoice_live/show.ex`:
- Around line 1111-1125: The new-comment UI uses a single-line <input> which
prevents multiline entry; change the form field in the "submit_comment" form to
a <textarea> that uses the same name/value attributes from `@comment_form` (i.e.
`@comment_form`[:body].name and `@comment_form`[:body].value) and keep the existing
id ("comment-form-#{`@comment_form_key`}"), phx-submit and layout; ensure the
textarea preserves newline behavior (e.g., add rows and matching classes as
needed) so creation matches the existing edit/multi-line rendering behavior.
In `@lib/ksef_hub_web/live/tag_live.ex`:
- Around line 174-197: Replace the raw <input> blocks for the name and
description fields with the shared .input component so styling, labels and error
rendering come from core_components; specifically swap the blocks that reference
`@form`[:name] and `@form`[:description] to use the .input component, passing the
form field (e.g. field={`@form`[:name]} and field={`@form`[:description]}), label,
placeholder and required where applicable, and remove the manual <.error> block
for the name field since .input handles errors automatically.
In `@lib/ksef_hub_web/live/token_live.ex`:
- Around line 147-150: The Cancel button only toggles show_create_form and
leaves the existing `@form` and changeset behind; update the handle_event for
"toggle_form" (the toggle_form code path in TokenLive) so that when hiding the
create card (i.e., changing show_create_form from true to false) you also clear
the `@form` assign (set form: nil or rebuild an empty form/changeset) so the next
open shows a fresh form with no errors or prior values.
In `@test/ksef_hub_web/live/invoice_live/index_test.exs`:
- Around line 214-227: The test currently only checks footer text but not that
navigation is disabled; update the "shows pagination footer with disabled nav
for single page" test to assert the disabled state of pagination controls by
inspecting the rendered HTML from live/3 (html): either assert the Previous and
Next controls have a disabled attribute or aria-disabled="true" (or a "disabled"
CSS class depending on implementation), or parse the HTML and assert there are
no anchor (<a>) links for the prev/next controls (e.g., check for absence of
href on elements labeled "Previous"/"Next" or absence of link elements with
data-testid for prev/next); reference the test block name and the pagination
markup assertions to locate and change the assertions.
---
Outside diff comments:
In `@lib/ksef_hub_web/components/layouts.ex`:
- Around line 66-80: The hidden return_to currently uses `@current_path` which can
point to another company's /c/:id/... URL causing a redirect back to the wrong
company; update the form in the company loop (the :for over `@companies`,
company.id and the hidden "return_to") to produce a target-company path instead
of blindly reusing `@current_path` — either rewrite the leading /c/:id/ segment of
`@current_path` to use the current company.id (when `@current_path` matches
^/c/\d+/) or build a safe fallback path for the company (e.g. invoices path) and
set that as the hidden return_to value so the switch-company action always
redirects to the same company selected in the form.
In `@lib/ksef_hub_web/live/invoice_live/index.ex`:
- Around line 44-80: filter_assigns/4 currently uses the raw filters to build
the form and active_filters which can show an unauthorized "type" chip;
normalize the filters[:type] at the top of filter_assigns by checking
Authorization.can?(role, :view_all_invoice_types) and, if the role cannot view
all types, clear or coerce filters[:type] to nil/"" (or an allowed default)
before you build the form (the "type" key passed into to_form) and before
calling build_active_filters so the UI no longer displays an unauthorized type
filter; update only the local filters variable used by the form and
build_active_filters (leave the original query semantics intact elsewhere).
In `@test/ksef_hub_web/live/invoice_live/show_test.exs`:
- Around line 186-190: The current test uses render_hook(view, "approve", %{})
and then only checks the absence of a success badge via has_element?, which
doesn't guarantee the invoice wasn't mutated; after calling render_hook in the
test (invoice_live/show_test.exs) fetch the invoice record from the database
(e.g., via Repo.get!/Repo.reload or the existing context function used
elsewhere) and assert its status field remains the original (unchanged) value,
and optionally assert the live view flash contains an error message if expected;
keep the render_hook(view, "approve", %{}) call and add the DB fetch + assert on
the persisted invoice status (and flash check) to prove the server-side guard
held.
---
Nitpick comments:
In `@assets/css/app.css`:
- Around line 201-240: Unquote the generic family name in the body font stack
and standardize quotes for the mono font: in the body rule (selector "body")
remove quotes around Geist so it reads Geist, and in the code/pre/kbd/.font-mono
rule replace the single-quoted 'Geist Mono' with double quotes "Geist Mono"
(keep the rest of the fallback families unchanged) so Stylelint no longer flags
the generic name and the multi-word font uses proper double quotes.
In `@lib/ksef_hub_web/components/core_components.ex`:
- Around line 320-324: The nav_active? function may produce unintended matches
for similarly prefixed paths; replace the current starts_with? check with a
boundary-aware match using Regex: update defp nav_active?(current, path) to
return true when current == path or when
Regex.match?(~r/^#{Regex.escape(path)}(\/|$)/, current) so only exact path or
path followed by a single slash/segment counts as active.
In `@lib/ksef_hub_web/components/layouts/root.html.heex`:
- Around line 10-15: The page currently loads Google Fonts via external
links/hints (the <link> tags that reference https://fonts.googleapis.com and
https://fonts.gstatic.com and the href with family=Geist and Geist+Mono);
replace these external references by self-hosting the Geist and Geist Mono font
files: download the needed font weights, add `@font-face` declarations in your app
stylesheet referencing local asset paths, update the HTML to point to the local
CSS/font files instead of the Google URLs, and remove the preconnect/link tags
to fonts.googleapis.com/fonts.gstatic.com so the app no longer depends on the
external CDN.
In `@lib/ksef_hub_web/controllers/page_html/home.html.heex`:
- Around line 25-36: Add unique DOM id attributes to the two action anchors for
testability: the link with href ~p"/users/log-in" (text "Log in") should get a
stable id such as "primary-cta-login" and the link with href ~p"/users/register"
(text "Create an account") should get a stable id such as
"primary-cta-register"; update the <a> elements in the home.html.heex template
that render those two buttons to include these id attributes so tests can
reliably select them (follow the existing pattern used for
id="primary-cta-companies").
- Line 5: Replace the raw span element that uses the hero-document-text CSS
class with the standard icon component: locate the <span
class="hero-document-text size-8 text-primary"> and swap it out to call the
.icon component from core_components (use the appropriate icon name mapping for
the "hero-document-text" glyph and pass the existing size-8 and text-primary
classes through the component's class prop) so the template uses .icon
consistently across the codebase.
In `@lib/ksef_hub_web/live/category_live.ex`:
- Around line 171-274: The render/1 is too large and mixes LiveView state with
UI; extract the form and table into two stateless function components
category_form/1 and category_table/1, moving the form markup (inputs using
`@form`, `@editing`, translate_error, phx-submit/validate/cancel_edit) into
category_form(assigns) and the table markup (rows=@streams.categories,
row_id/row_item, action buttons with phx-click edit/delete and data-confirm)
into category_table(assigns); then simplify render/1 to call ~H{
category_form(`@assigns`) category_table(`@assigns`) } and ensure both components
accept and use the same assigns they reference (form, editing, streams, etc.) so
event bindings and helpers continue to work.
In `@test/ksef_hub_web/live/team_live_test.exs`:
- Line 104: The assertion in test/ksef_hub_web/live/team_live_test.exs uses
has_element?(view, "[data-testid='team-table'] [data-role='reviewer']") which is
too broad; update the selector used in the has_element? call to scope it to the
pending invitations stream (anchor to the pending-invitations-list element ID
you added) so it targets only the pending invitation row (e.g., prepend the
"#pending-invitations-list" id to the selector) and leave the rest of the test
logic unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d777450e-7eb1-46c7-8102-25bb9900546e
📒 Files selected for processing (32)
assets/css/app.csslib/ksef_hub_web/components/core_components.exlib/ksef_hub_web/components/invoice_components.exlib/ksef_hub_web/components/layouts.exlib/ksef_hub_web/components/layouts/root.html.heexlib/ksef_hub_web/controllers/page_controller.exlib/ksef_hub_web/controllers/page_html/home.html.heexlib/ksef_hub_web/live/category_live.exlib/ksef_hub_web/live/certificate_live.exlib/ksef_hub_web/live/company_live/index.exlib/ksef_hub_web/live/dashboard_live.exlib/ksef_hub_web/live/export_live/index.exlib/ksef_hub_web/live/invitation_accept_live.exlib/ksef_hub_web/live/invoice_live/index.exlib/ksef_hub_web/live/invoice_live/public_show.exlib/ksef_hub_web/live/invoice_live/show.exlib/ksef_hub_web/live/invoice_live/upload.exlib/ksef_hub_web/live/sync_live.exlib/ksef_hub_web/live/tag_live.exlib/ksef_hub_web/live/team_live.exlib/ksef_hub_web/live/token_live.exlib/ksef_hub_web/live/user_confirmation_instructions_live.exlib/ksef_hub_web/live/user_confirmation_live.exlib/ksef_hub_web/live/user_forgot_password_live.exlib/ksef_hub_web/live/user_login_live.exlib/ksef_hub_web/live/user_registration_live.exlib/ksef_hub_web/live/user_reset_password_live.extest/ksef_hub_web/controllers/page_controller_test.exstest/ksef_hub_web/live/dashboard_live_test.exstest/ksef_hub_web/live/invoice_live/index_test.exstest/ksef_hub_web/live/invoice_live/show_test.exstest/ksef_hub_web/live/team_live_test.exs
- Fix label/input association in category_live (add id/for attributes) - Convert empty emoji/description to nil in category atomize_params - Align cert expiry threshold to 7 days in certificate_live - Show Expired badge on dashboard when cert is past expiry - Make export form submit both Preview and Export via _action param - Replace comment input with textarea for multiline support - Use .input component in tag_live for name/description fields - Reset form on token_live cancel to clear stale state - Fix company switcher return_to to rewrite path for target company - Normalize unauthorized type filter in invoice_live/index - Add DB assertion in show_test for approve-on-income guard - Assert disabled pagination controls in index_test - Scope team_live_test selector to pending-invitations-list - Fix font quoting in CSS (unquote Geist, double-quote Geist Mono) - Use boundary-aware regex in nav_active? to prevent prefix collisions - Add stable test IDs to home page CTA links Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (3)
lib/ksef_hub_web/live/category_live.ex (1)
192-246: Consider using the<.input>component for form inputs.The form uses raw
<input>elements with manualid,name, andvaluebindings. Per coding guidelines, the<.input>component fromcore_components.exhandles labels, error display, and consistent styling automatically.The current implementation is functional, but switching to
<.input>would reduce boilerplate and ensure consistency:- <div> - <label for={`@form`[:emoji].id} class="block text-xs text-muted-foreground mb-1"> - Emoji - </label> - <input - type="text" - id={`@form`[:emoji].id} - name={`@form`[:emoji].name} - value={`@form`[:emoji].value} - placeholder="📦" - class="h-9 w-16 rounded-md border border-input bg-background px-3 text-sm focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring text-center" - /> - </div> + <div> + <.input field={`@form`[:emoji]} label="Emoji" placeholder="📦" class="w-16 text-center" /> + </div>As per coding guidelines: "Always use the imported
<.input>component for form inputs fromcore_components.exwhen available."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ksef_hub_web/live/category_live.ex` around lines 192 - 246, Replace the raw <input> tags for the fields emoji, name, description, and sort_order with the imported <.input> component so IDs, names, values, labels and error rendering are handled consistently; for each field use form={`@form`} and field={:emoji}, field={:name}, field={:description}, field={:sort_order} (preserve attributes like placeholder and required for :name), remove the manual <.error> for `@form`[:name] (the component will render errors via translate_error), and keep the same classes/props via the component's class/opts to maintain styling and behavior.lib/ksef_hub_web/controllers/page_html/home.html.heex (1)
1-14: Consider using the<.icon>component for the hero icon.Line 5 uses a raw
<span>with a hero icon class. Per coding guidelines, use the<.icon>component instead:- <span class="hero-document-text size-8 text-primary"></span> + <.icon name="hero-document-text" class="size-8 text-primary" />As per coding guidelines: "Always use the
<.icon>component for icons from core_components.ex, never useHeroiconsmodules or similar."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ksef_hub_web/controllers/page_html/home.html.heex` around lines 1 - 14, Replace the raw <span> element that uses the hero icon class ("hero-document-text") with the project's <.icon> component from core_components (use the icon name "document-text") and pass the same styling classes (e.g. size-8 and text-primary) to it; update the markup where the span with class "hero-document-text" is rendered so it uses <.icon name="document-text" class="size-8 text-primary" /> instead of the span.lib/ksef_hub_web/live/invoice_live/index.ex (1)
47-92: Cyclomatic complexity exceeds threshold (10 > 9).The pipeline reports that
filter_assigns/4has a cyclomatic complexity of 10, exceeding the configured maximum of 9. Consider extracting helper functions to reduce complexity:♻️ Suggested refactor to reduce complexity
+ `@spec` normalize_filters(map(), atom() | nil) :: map() + defp normalize_filters(filters, role) do + if filters[:type] && !Authorization.can?(role, :view_all_invoice_types) do + Map.delete(filters, :type) + else + filters + end + end + + `@spec` build_filter_form(map()) :: Phoenix.HTML.Form.t() + defp build_filter_form(filters) do + %{ + "type" => to_string_or_empty(filters[:type]), + "status" => to_string_or_empty(filters[:status]), + "date_from" => (filters[:date_from] && Date.to_iso8601(filters[:date_from])) || "", + "date_to" => (filters[:date_to] && Date.to_iso8601(filters[:date_to])) || "", + "query" => filters[:query] || "", + "category_id" => filters[:category_id] || "", + "tag_id" => first_tag_id(filters) || "" + } + |> to_form(as: :filters) + end + `@spec` filter_assigns(map(), map(), atom() | nil, map()) :: keyword() defp filter_assigns(filters, result, role, assigns) do - normalized_filters = - if filters[:type] && !Authorization.can?(role, :view_all_invoice_types) do - Map.delete(filters, :type) - else - filters - end - - form = - %{ - "type" => to_string_or_empty(normalized_filters[:type]), - ... - } - |> to_form(as: :filters) + normalized_filters = normalize_filters(filters, role) + form = build_filter_form(normalized_filters) active_filters = build_active_filters( normalized_filters, Map.get(assigns, :categories, []), Map.get(assigns, :all_tags, []) ) [ invoices: result.entries, filters: filters, form: form, ... ] end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ksef_hub_web/live/invoice_live/index.ex` around lines 47 - 92, filter_assigns/4 is over the cyclomatic complexity limit; extract parts into small helpers to reduce branching inside filter_assigns. Move the type-normalization logic into a new normalize_filters(filters, role) function that uses Authorization.can?(role, :view_all_invoice_types), move the form building block (currently creating the map with "type","status","date_from", etc. and to_form(as: :filters)) into build_filters_form(normalized_filters), and optionally extract the active_filters construction into build_active_filters_wrapper(normalized_filters, assigns) that delegates to the existing build_active_filters/3; keep filter_assigns/4 only orchestration that calls normalize_filters/2, build_filters_form/1, build_active_filters_wrapper/3 and returns the final keyword list (preserving keys like invoices, filters, form, page, per_page, total_count, total_pages, can_view_all_types, can_create, active_filters, filter_count).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/ksef_hub_web/controllers/page_html/home.html.heex`:
- Around line 1-14: Replace the raw <span> element that uses the hero icon class
("hero-document-text") with the project's <.icon> component from core_components
(use the icon name "document-text") and pass the same styling classes (e.g.
size-8 and text-primary) to it; update the markup where the span with class
"hero-document-text" is rendered so it uses <.icon name="document-text"
class="size-8 text-primary" /> instead of the span.
In `@lib/ksef_hub_web/live/category_live.ex`:
- Around line 192-246: Replace the raw <input> tags for the fields emoji, name,
description, and sort_order with the imported <.input> component so IDs, names,
values, labels and error rendering are handled consistently; for each field use
form={`@form`} and field={:emoji}, field={:name}, field={:description},
field={:sort_order} (preserve attributes like placeholder and required for
:name), remove the manual <.error> for `@form`[:name] (the component will render
errors via translate_error), and keep the same classes/props via the component's
class/opts to maintain styling and behavior.
In `@lib/ksef_hub_web/live/invoice_live/index.ex`:
- Around line 47-92: filter_assigns/4 is over the cyclomatic complexity limit;
extract parts into small helpers to reduce branching inside filter_assigns. Move
the type-normalization logic into a new normalize_filters(filters, role)
function that uses Authorization.can?(role, :view_all_invoice_types), move the
form building block (currently creating the map with
"type","status","date_from", etc. and to_form(as: :filters)) into
build_filters_form(normalized_filters), and optionally extract the
active_filters construction into
build_active_filters_wrapper(normalized_filters, assigns) that delegates to the
existing build_active_filters/3; keep filter_assigns/4 only orchestration that
calls normalize_filters/2, build_filters_form/1, build_active_filters_wrapper/3
and returns the final keyword list (preserving keys like invoices, filters,
form, page, per_page, total_count, total_pages, can_view_all_types, can_create,
active_filters, filter_count).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 32a4be29-4834-4361-a560-15dbf9a28af2
📒 Files selected for processing (16)
assets/css/app.csslib/ksef_hub_web/components/core_components.exlib/ksef_hub_web/components/layouts.exlib/ksef_hub_web/controllers/page_html/home.html.heexlib/ksef_hub_web/live/category_live.exlib/ksef_hub_web/live/certificate_live.exlib/ksef_hub_web/live/dashboard_live.exlib/ksef_hub_web/live/export_live/index.exlib/ksef_hub_web/live/invoice_live/index.exlib/ksef_hub_web/live/invoice_live/show.exlib/ksef_hub_web/live/tag_live.exlib/ksef_hub_web/live/token_live.extest/ksef_hub_web/live/export_live/index_test.exstest/ksef_hub_web/live/invoice_live/index_test.exstest/ksef_hub_web/live/invoice_live/show_test.exstest/ksef_hub_web/live/team_live_test.exs
- Replace raw <span> hero icon with <.icon> component in home page - Replace raw <input> tags with <.input> component in category form - Extract normalize_filters/2 and build_filters_form/1 from filter_assigns/4 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
lib/ksef_hub_web/live/invoice_live/index.ex (2)
148-153: Consider prepending instead of appending for list construction.Using
acc ++ [element]is O(n) per append. While the filter count is small (≤6), prepending and reversing is more idiomatic in Elixir.♻️ Suggested improvement
defp maybe_add_chip(acc, nil, _key, _label, _formatter), do: acc defp maybe_add_chip(acc, value, key, label, formatter) do - acc ++ [%{key: key, label: label, value: formatter.(value)}] + [%{key: key, label: label, value: formatter.(value)} | acc] endThen in
build_active_filters/3, add|> Enum.reverse()at the end if ordering matters for display.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ksef_hub_web/live/invoice_live/index.ex` around lines 148 - 153, The current maybe_add_chip/5 uses acc ++ [element] which is O(n); change it to prepend the new map with [%{key: key, label: label, value: formatter.(value)} | acc] in the maybe_add_chip function (preserving the guard clause for nil), and then ensure the caller build_active_filters/3 (or wherever the accumulator is collected) calls Enum.reverse() on the final result to restore the original order for display.
279-285: Add debounce to filter form to reduce server load.The form triggers
phx-change="filter"on every keystroke without debounce, causing excessive database queries during typing. This is especially impactful for the search input.♻️ Suggested fix
-<.form for={`@form`} phx-change="filter" class="contents"> +<.form for={`@form`} phx-change="filter" phx-debounce="300" class="contents">Based on learnings: "Debounce user input for search and filtering in LiveView"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ksef_hub_web/live/invoice_live/index.ex` around lines 279 - 285, The form currently fires phx-change="filter" on every keystroke; add debounce to the filter inputs so LiveView only sends the filter event after the user pauses typing. Locate the form and the filter inputs (the <.form ... phx-change="filter"> and the search inputs rendered by the <.filter_bar> component) and add phx-debounce (e.g., "300" or "500") to the search input(s) or the form so the "filter" event is throttled; ensure the change remains wired to the same "filter" event handler and update the <.filter_bar> props if you need to pass down the debounce value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/ksef_hub_web/live/invoice_live/index.ex`:
- Around line 148-153: The current maybe_add_chip/5 uses acc ++ [element] which
is O(n); change it to prepend the new map with [%{key: key, label: label, value:
formatter.(value)} | acc] in the maybe_add_chip function (preserving the guard
clause for nil), and then ensure the caller build_active_filters/3 (or wherever
the accumulator is collected) calls Enum.reverse() on the final result to
restore the original order for display.
- Around line 279-285: The form currently fires phx-change="filter" on every
keystroke; add debounce to the filter inputs so LiveView only sends the filter
event after the user pauses typing. Locate the form and the filter inputs (the
<.form ... phx-change="filter"> and the search inputs rendered by the
<.filter_bar> component) and add phx-debounce (e.g., "300" or "500") to the
search input(s) or the form so the "filter" event is throttled; ensure the
change remains wired to the same "filter" event handler and update the
<.filter_bar> props if you need to pass down the debounce value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4a3b5a80-9c1a-41d8-a006-062c405b989b
📒 Files selected for processing (3)
lib/ksef_hub_web/controllers/page_html/home.html.heexlib/ksef_hub_web/live/category_live.exlib/ksef_hub_web/live/invoice_live/index.ex
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/ksef_hub_web/controllers/page_html/home.html.heex
Replace O(n) `acc ++ [element]` with O(1) `[element | acc]` and Enum.reverse() at the end to preserve display order. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
1 participant
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
Refactor
Style / Bug Fixes
Tests