Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/components/content_card_component.html.erb
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<div class="card shadow-sm w-full border border-base-300 p-4 bg-base-200 bg-opacity-70
<div class="card shadow-sm w-full border border-base-300 p-2 sm:p-4 bg-base-200 bg-opacity-70
[background-color:var(--color-base-200)]
[background-image:radial-gradient(color-mix(in_srgb,var(--color-primary)_25%,transparent)_0.5px,transparent_0.5px)]
[background-size:10px_10px]">
<div class="card-body">
<div class="card-body p-2 sm:p-4">
<%= content %>
</div>
</div>
6 changes: 3 additions & 3 deletions app/components/modal_component.html.erb
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
<div id="modal"
class="fixed inset-0 z-50 hidden bg-black/40 backdrop-blur-sm flex items-center justify-center"
class="fixed inset-0 z-50 hidden bg-black/40 backdrop-blur-sm flex items-center justify-center p-4"
data-controller="modal"
data-action="turbo:submit-end->modal#close">

<div class="card bg-base-100 border border-base-300 rounded-xl shadow-xl w-full max-w-xl p-6 max-h-[90vh] flex flex-col">
<div class="card bg-base-100 border border-base-300 rounded-xl shadow-xl w-full max-w-xl p-4 sm:p-6 max-h-[90vh] flex flex-col">
<div class="flex items-center justify-between">
<h2 class="text-lg font-semibold"><%= title %></h2>
<h2 class="text-base sm:text-lg font-semibold"><%= title %></h2>
<button type="button"
class="btn btn-ghost btn-xs btn-circle text-base-content/40 hover:text-error"
data-action="modal#close">
Expand Down
8 changes: 4 additions & 4 deletions app/components/projects/log_schema_component.html.erb
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
<div class="flex justify-between items-center">
<div class="flex items-center space-x-2">
<h3 class="text-2xl font-bold">
<div class="flex flex-col sm:flex-row sm:justify-between sm:items-center gap-3">
<div class="flex items-center gap-2 flex-wrap">
<h3 class="text-xl sm:text-2xl font-bold">
<%= t(".title") %>
</h3>
<%= render BadgeComponent.new(text: t(".attributes_count", count: project.log_schema.size), colour: :warning, size: :sm) %>
</div>
<div>
<div class="flex-shrink-0">
<%= render ActionButtonComponent.new(to: edit_project_attributes_schema_path(project), icon: "edit", colour: :info, turbo_stream: true, size: :medium) do %>
<%= t(".edit_attributes") %>
<% end %>
Expand Down
8 changes: 4 additions & 4 deletions app/components/projects/projects_card_component.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<input id="search" type="search" name="q" required placeholder="<%= t(".search_placeholder") %>" autocomplete="off" data-project-card-search-target="input" data-action="input->project-card-search#filter">
</label>
</div>
<div class="grid grid-cols-[repeat(auto-fill,minmax(285px,1fr))] gap-6 w-full">
<div class="grid grid-cols-1 sm:grid-cols-2 lg:grid-cols-[repeat(auto-fill,minmax(285px,1fr))] gap-4 sm:gap-6 w-full">
<% projects.each do |project| %>
<div
class="card w-full flex flex-col justify-between gap-2 bg-base-100 border border-base-300 shadow-md p-4 rounded-xl"
Expand All @@ -15,10 +15,10 @@
<div class="flex flex-col">
<div class="flex justify-between items-center pb-2 mb-2">
<h2
class="text-color-base-content text-lg font-semibold">
class="text-color-base-content text-base sm:text-lg font-semibold truncate pr-2">
<%= project.name %>
</h2>
<div>
<div class="flex-shrink-0">
<%= render BadgeComponent.new(**project_badge_args(project), style: :soft, size: :sm) %>
</div>
</div>
Expand All @@ -28,7 +28,7 @@
<% end %>
</div>
</div>
<%= link_to t(".view_project"), project_path(project), class: "text-sm bg-base-300 rounded-md py-2 flex flex-row justify-center items-center" %>
<%= link_to t(".view_project"), project_path(project), class: "text-sm bg-base-300 hover:bg-base-400 rounded-md py-2 flex flex-row justify-center items-center transition-colors" %>
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The transition-colors class is added without a corresponding hover: effect already existing on the element. While hover:bg-base-400 is added, the transition-colors class would provide a smoother transition. However, this is only effective if there are color changes to transition. Consider also adding a transition duration class like duration-200 or duration-300 for a more polished hover effect.

Copilot uses AI. Check for mistakes.
</div>
<% end %>
</div>
Expand Down
50 changes: 36 additions & 14 deletions app/components/shared/index_table_component.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -17,29 +17,51 @@
<!-- Construct columns sizes string from individual col_size parameters -->
<% column_sizes = columns.map { |c| c.col_size || "minmax(0, 1fr)" }.join(" ") %>

<!-- Each row (including header) is its own rounded box -->
<div class="mb-2">
<div class="card bg-neutral rounded-lg">
<div class="p-4 grid items-center gap-4" style="grid-template-columns: <%= column_sizes %>">
<% columns.each do |column| %>
<div class="text-sm font-semibold text-neutral-content"><%= column.header %></div>
<% end %>
<!-- Desktop view: Grid layout -->
<div class="hidden md:block">
<!-- Each row (including header) is its own rounded box -->
<div class="mb-2">
<div class="card bg-neutral rounded-lg">
<div class="p-4 grid items-center gap-4" style="grid-template-columns: <%= column_sizes %>">
<% columns.each do |column| %>
<div class="text-sm font-semibold text-neutral-content"><%= column.header %></div>
<% end %>
</div>
</div>
</div>

<div class="space-y-2">
<% records.each_with_index do |record, index| %>
<div data-index-table-component-target="row" class="card bg-white rounded-lg shadow-sm">
<div class="p-3 grid items-center gap-4" style="grid-template-columns: <%= column_sizes %>">
<% columns.each do |column| %>
<div class="text-sm text-base-content"><%= render_cell_content(record, column) %></div>
<% end %>
</div>
</div>
<% end %>
Comment on lines +34 to +42
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The desktop and mobile views both use data-index-table-component-target="row", which means the existing JavaScript filter will apply to both versions simultaneously. The filter uses inline display styles which can conflict with Tailwind's responsive display classes (hidden md:block and md:hidden). Consider using separate target names (e.g., "desktopRow" and "mobileRow") or ensuring the controller handles responsive duplicates correctly.

Copilot uses AI. Check for mistakes.
<div data-index-table-component-target="emptyRow" class="card bg-white rounded-lg shadow-sm <%= "hidden" unless empty? %>">
<div class="flex justify-center p-3 items-center">
<div class="text-sm text-base-content/40 font-bold"><%= t("shared.index_table_component.no_entries") %></div>
</div>
</div>
</div>
</div>

<div class="space-y-2">
<!-- Mobile view: Stacked cards -->
<div class="md:hidden space-y-3">
<% records.each_with_index do |record, index| %>
<div data-index-table-component-target="row" class="card bg-white rounded-lg shadow-sm">
<div class="p-3 grid items-center gap-4" style="grid-template-columns: <%= column_sizes %>">
<% columns.each do |column| %>
<div data-index-table-component-target="row" class="card bg-white rounded-lg shadow-sm p-4">
<% columns.each do |column| %>
<div class="flex flex-col py-2 border-b border-base-200 last:border-0">
<div class="text-xs font-semibold text-base-content mb-1"><%= column.header %></div>
<div class="text-sm text-base-content"><%= render_cell_content(record, column) %></div>
<% end %>
</div>
</div>
<% end %>
</div>
<% end %>
Comment on lines +34 to 62
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The data-index-table-component-target="row" attribute is used on both desktop and mobile row elements, causing the JavaScript controller to select all rows (both visible and hidden due to responsive classes). When the filter function runs, it will manipulate inline styles on both sets of rows. While this might work, it's inefficient as it processes twice as many DOM elements as necessary. Additionally, if the filter sets display: none on hidden mobile rows and then the viewport is resized to mobile, those rows won't appear even though they should. Consider using separate target names like desktopRow and mobileRow, or refactoring to a single row template that changes layout responsively via CSS only.

Copilot uses AI. Check for mistakes.
<div data-index-table-component-target="emptyRow" class="card bg-white rounded-lg shadow-sm <%= "hidden" unless empty? %>">
<div class="flex justify-center p-3 items-center">
<div class="flex justify-center p-4 items-center">
<div class="text-sm text-base-content/40 font-bold"><%= t("shared.index_table_component.no_entries") %></div>
</div>
</div>
Comment on lines +20 to 67
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicating the entire table structure for mobile and desktop views doubles the number of DOM elements, which could impact performance for large datasets. Each record creates two complete row elements (one for desktop, one for mobile) even though only one is visible at a time. For tables with many records or columns, consider a CSS-only responsive approach that transforms a single table structure, or implement lazy rendering where only the visible view's elements are generated based on viewport size.

Copilot uses AI. Check for mistakes.
Comment on lines +43 to 67
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The data-index-table-component-target="emptyRow" is duplicated in both desktop and mobile views, but the controller expects a single emptyRow target (it uses this.emptyRowTarget singular). Stimulus will only return the first matching element when using the singular form, which means the empty state will only work correctly in one view (desktop). The mobile empty state will not be properly controlled by the JavaScript filter function. Consider using a single shared empty row element or handling both empty states in the controller.

Copilot uses AI. Check for mistakes.
Expand Down
6 changes: 3 additions & 3 deletions app/components/sidebar_component.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<ul class="menu shadow-lg bg-base-200 w-64 min-h-screen tracking-wide justify-between">
<ul data-mobile-menu-target="sidebar" role="navigation" aria-label="Main navigation" class="menu shadow-lg bg-base-200 w-64 min-h-screen tracking-wide justify-between fixed md:sticky top-0 left-0 z-40 transition-transform duration-300 -translate-x-full md:translate-x-0">
<div>
<div class="px-4 py-4 border-b border-base-300">
<%= link_to root_path do %>
Expand All @@ -10,12 +10,12 @@
</div>
<% end %>
</div>
<li class="py-6">
<li class="py-6 overflow-y-auto max-h-[calc(100vh-200px)]">
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The overflow-y-auto and max-h-[calc(100vh-200px)] classes are applied to the <li> element, but these classes should typically be applied to a containing <div> or <ul> element. The <li> element is a list item and applying scroll behavior directly to it is semantically incorrect and may cause unexpected behavior across different browsers. Consider wrapping the sections in a scrollable div instead.

Copilot uses AI. Check for mistakes.
<% sections.each do |section| %>
<h2 class="menu-title uppercase"> <%= section.title %></h2>
<ul>
<% section.items.each do |item| %>
<li> <%= link_to item.path do %>
<li> <%= link_to item.path, data: { action: "click->mobile-menu#close" } do %>
<%= render IconComponent.new(icon: item.icon, size: :md, classes: "w-5 h-5") %>
<span class="truncate text-base-content"><%= item.name %></span>
<% end %>
Expand Down
17 changes: 9 additions & 8 deletions app/components/users/form_component.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,16 @@
<%= form.label :name %>
<%= form.text_field :name, required: true, class: "w-full", placeholder: t(".placeholder") %>
</div>
<div class="flex gap-4">
<div class="flex flex-col gap-1">
<%= form.label :roles %>
<% UserRole.roles.each_key do |role| %>
<label class="flex items-center gap-2">
<%= check_box_tag("user[roles][]", role, @user.has_roles?(role.to_sym), class: "checkbox") %>
<%= role.humanize %>
</label>
<% end %>

<div class="flex flex-wrap gap-3 sm:gap-4">
<% UserRole.roles.each_key do |role| %>
<label class="flex items-center gap-2 text-sm sm:text-base">
<%= check_box_tag("user[roles][]", role, @user.has_roles?(role.to_sym), class: "checkbox") %>
<%= role.humanize %>
</label>
<% end %>
</div>
</div>
<div class="flex flex-col gap-1">
<%= form.label :password %>
Expand Down
45 changes: 45 additions & 0 deletions app/javascript/controllers/mobile_menu_controller.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { Controller } from "@hotwired/stimulus";

// Mobile menu controller for toggling sidebar on mobile devices
export default class extends Controller {
static targets = ["sidebar", "overlay", "menuButton"];

connect() {
// Close menu on window resize if screen becomes larger
this.handleResize = this.handleResize.bind(this);
window.addEventListener("resize", this.handleResize);
}

disconnect() {
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential memory leak: if the controller is disconnected while the menu is open, document.body.style.overflow will remain set to "hidden", preventing scrolling on the page. Consider calling this.close() in the disconnect() method to ensure cleanup of body styles when the controller is removed from the DOM.

Suggested change
disconnect() {
disconnect() {
this.close();

Copilot uses AI. Check for mistakes.
window.removeEventListener("resize", this.handleResize);
}

toggle() {
const isOpen = !this.sidebarTarget.classList.contains("-translate-x-full");

if (isOpen) {
this.close();
} else {
this.open();
}
}

open() {
this.sidebarTarget.classList.remove("-translate-x-full");
this.overlayTarget.classList.remove("hidden");
document.body.style.overflow = "hidden";
}

close() {
this.sidebarTarget.classList.add("-translate-x-full");
this.overlayTarget.classList.add("hidden");
document.body.style.overflow = "";
}

handleResize() {
// Close menu if screen becomes large (md breakpoint = 768px)
if (window.innerWidth >= 768) {
this.close();
}
}
}
20 changes: 11 additions & 9 deletions app/views/journals/show.html.erb
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
<div class="mx-8 my-10">
<div class="flex items-center justify-between mb-5">
<h1 class="text-4xl font-bold"><%= @journal.title %></h1>
<div class="flex flex-row gap-4">
<div class="mx-4 sm:mx-8 my-6 sm:my-10">
<div class="flex flex-col gap-4 mb-5">
<h1 class="text-2xl sm:text-3xl lg:text-4xl font-bold"><%= @journal.title %></h1>
<div class="flex flex-row flex-wrap gap-2 sm:gap-4">
<%= render ActionButtonComponent.new(to: edit_project_subproject_journal_path(@project, @subproject, @journal), icon: "edit", colour: :info, size: :large) do %>
<%= t(".actions.edit") %>
<span class="hidden sm:inline"><%= t(".actions.edit") %></span>
<span class="sm:hidden text-xs whitespace-nowrap"><%= t(".actions.edit") %></span>
<% end %>
<%= render ActionButtonComponent.new(to: project_subproject_journal_path(@project, @subproject, @journal), method: :delete, icon: "delete", colour: :error, size: :large, confirm: t("journals.destroy.confirm")) do %>
<%= t(".actions.delete") %>
<span class="hidden sm:inline"><%= t(".actions.delete") %></span>
<span class="sm:hidden text-xs whitespace-nowrap"><%= t(".actions.delete") %></span>
<% end %>
</div>
</div>
<div class="flex flex-col">
<span class="text-lg text-base-content/60"><%= t(".created_on") %>: <%= @journal.created_at.to_fs(:long_ordinal) %></span>
<span class="text-lg text-base-content/60"><%= t(".author") %>: <%= @journal.user.username %></span>
<div class="flex flex-col gap-1">
<span class="text-base sm:text-lg text-base-content/60"><%= t(".created_on") %>: <%= @journal.created_at.to_fs(:long_ordinal) %></span>
<span class="text-base sm:text-lg text-base-content/60"><%= t(".author") %>: <%= @journal.user.username %></span>
</div>
<div class="divider m-0 mb-2"></div>
<%= @journal.markdown_content %>
Expand Down
22 changes: 20 additions & 2 deletions app/views/layouts/application.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,28 @@
</head>

<body class="min-h-screen bg-base-100">
<div class="flex min-h-screen">
<div class="flex min-h-screen" <%= "data-controller=\"mobile-menu\"".html_safe unless current_page?(login_path) %>>
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of string interpolation with .html_safe for adding data attributes is not idiomatic and unnecessarily complex. Consider using Rails' tag helpers or conditional attribute hashing instead. For example, you could conditionally add the data-controller attribute using a ternary operator within the HTML tag itself, or use content_tag.

Copilot uses AI. Check for mistakes.
<!-- Mobile Menu Button (visible only on mobile and when not on login page) -->
<% unless current_page?(login_path) %>
<button
data-mobile-menu-target="menuButton"
data-action="click->mobile-menu#toggle"
class="md:hidden fixed top-4 left-4 z-50 btn btn-circle btn-primary shadow-lg">
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" class="inline-block h-5 w-5 stroke-current">
<path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M4 6h16M4 12h16M4 18h16"></path>
</svg>
</button>

<!-- Overlay (for mobile when menu is open) -->
<div
data-mobile-menu-target="overlay"
data-action="click->mobile-menu#close"
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The overlay div should have aria-hidden="true" to prevent screen readers from announcing it, as it's purely a visual element. Additionally, when the overlay is visible (menu is open), focus should be trapped within the sidebar menu to prevent keyboard users from accidentally tabbing to elements behind the overlay. Consider implementing focus trap functionality in the mobile menu controller.

Suggested change
data-action="click->mobile-menu#close"
data-action="click->mobile-menu#close"
aria-hidden="true"

Copilot uses AI. Check for mistakes.
class="hidden fixed inset-0 bg-black/50 z-30 md:hidden"></div>
<% end %>

<%= render SidebarComponent.new %>

<main class="flex-1 overflow-auto">
<main class="flex-1 overflow-auto <%= current_page?(login_path) ? "" : "pt-16 md:pt-0" %>">
<%= render "shared/flash" %>
<%= yield %>
<div id="modal"></div>
Expand Down
21 changes: 12 additions & 9 deletions app/views/projects/show.html.erb
Original file line number Diff line number Diff line change
@@ -1,28 +1,31 @@
<section class="mx-8 my-10">
<section class="mx-4 sm:mx-8 my-6 sm:my-10">
<div class="flex flex-col gap-4">
<div class="flex flex-row justify-between items-start">
<div class="flex flex-col sm:flex-row sm:justify-between sm:items-start gap-4">
<div class="flex flex-col gap-1">
<h1 class="text-4xl font-bold"><%= @project.name %></h1>
<span class="text-lg text-base-content/60"><%= @project.description %></span>
<h1 class="text-2xl sm:text-3xl lg:text-4xl font-bold"><%= @project.name %></h1>
<span class="text-base sm:text-lg text-base-content/60"><%= @project.description %></span>
</div>

<div class="flex flex-row gap-4">
<div class="flex flex-row flex-wrap gap-2 sm:gap-4 sm:flex-shrink-0">
<%= render ActionButtonComponent.new(to: edit_project_path(@project), icon: "edit", colour: :info, turbo_stream: true, size: :large) do %>
<%= t(".actions.edit") %>
<span class="hidden sm:inline"><%= t(".actions.edit") %></span>
<span class="sm:hidden text-xs whitespace-nowrap"><%= t(".actions.edit") %></span>
<% end %>

<%= render ActionButtonComponent.new(to: new_project_subproject_path(@project), icon: "add", size: :large, turbo_stream: true, colour: :primary) do %>
<%= t(".create_subproject") %>
<span class="hidden sm:inline"><%= t(".create_subproject") %></span>
<span class="sm:hidden text-xs whitespace-nowrap"><%= t(".create_subproject") %></span>
<% end %>

<%= render ActionButtonComponent.new(to: project_path(@project), method: :delete, icon: "delete", colour: :error, size: :large, confirm: t("projects.destroy.confirm")) do %>
<%= t(".actions.delete") %>
<span class="hidden sm:inline"><%= t(".actions.delete") %></span>
<span class="sm:hidden text-xs whitespace-nowrap"><%= t(".actions.delete") %></span>
<% end %>
</div>
</div>

<%= render ContentCardComponent.new do %>
<h1 class="text-2xl font-bold"><%= t(".subprojects") %></h1>
<h1 class="text-xl sm:text-2xl font-bold"><%= t(".subprojects") %></h1>
<%= render Shared::IndexTableComponent.new(records: @project.subprojects) do |table| %>
<% table.column :name do |subproject| %>
<%= subproject.name %>
Expand Down
9 changes: 5 additions & 4 deletions app/views/regions/index.html.erb
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
<div class="mx-8 my-10">
<div class="flex items-center justify-between mb-5">
<h1 class="text-4xl font-bold"><%= t(".title") %></h1>
<div class="mx-4 sm:mx-8 my-6 sm:my-10">
<div class="flex flex-col sm:flex-row sm:items-center sm:justify-between gap-4 mb-5">
<h1 class="text-2xl sm:text-3xl lg:text-4xl font-bold"><%= t(".title") %></h1>
<%= render ActionButtonComponent.new(to: new_region_path, icon: "add", colour: :primary, size: :large, turbo_stream: true) do %>
<%= t(".actions.create") %>
<span class="hidden sm:inline"><%= t(".actions.create") %></span>
<span class="sm:hidden">New</span>
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoded text "New" should be internationalized. The full button text uses t(".actions.create") which supports multiple languages, but the mobile abbreviated text is hardcoded in English. This creates inconsistent internationalization and will not translate for non-English users. Consider adding an i18n key like t(".actions.create_short") or similar.

Suggested change
<span class="sm:hidden">New</span>
<span class="sm:hidden"><%= t(".actions.create_short") %></span>

Copilot uses AI. Check for mistakes.
<% end %>
</div>
<%= render ContentCardComponent.new do %>
Expand Down
8 changes: 5 additions & 3 deletions app/views/sessions/login.html.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<fieldset class="fieldset bg-base-200 border-base-300 rounded-box w-xs border p-4 mt-10 mx-auto">
<legend class="fieldset-legend text-lg font-semibold">Login</legend>
<div class="flex justify-center items-start min-h-screen px-4 py-10">
<fieldset class="fieldset bg-base-200 border-base-300 rounded-box w-full max-w-sm border p-4 sm:p-6">
<legend class="fieldset-legend text-base sm:text-lg font-semibold">Login</legend>

<%= form_with url: login_path, local: true do |form| %>
<div>
Expand All @@ -16,4 +17,5 @@
<%= form.submit "Login", class: "btn btn-neutral w-full" %>
</div>
<% end %>
</fieldset>
</fieldset>
</div>
Loading