feat: implement mobile responsiveness on all pages #223
There are no files selected for viewing
| 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-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 p-2 sm:p-4"> | ||
| <%= content %> | ||
| </div> | ||
| </div> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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-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" | ||
|
|
@@ -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-base sm:text-lg font-semibold truncate pr-2"> | ||
| <%= project.name %> | ||
| </h2> | ||
| <div class="flex-shrink-0"> | ||
| <%= render BadgeComponent.new(**project_badge_args(project), style: :soft, size: :sm) %> | ||
| </div> | ||
| </div> | ||
|
|
@@ -28,7 +28,7 @@ | |
| <% end %> | ||
| </div> | ||
| </div> | ||
| <%= 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" %> | ||
|
There was a problem hiding this comment. The |
||
| </div> | ||
| <% end %> | ||
| </div> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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(" ") %> | ||
|
|
||
| <!-- 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
There was a problem hiding this comment. 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. |
||
| <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> | ||
|
|
||
| <!-- 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 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> | ||
| </div> | ||
| <% end %> | ||
| </div> | ||
| <% end %> | ||
|
Comment on lines
+34
to
62
There was a problem hiding this comment. The |
||
| <div data-index-table-component-target="emptyRow" class="card bg-white rounded-lg shadow-sm <%= "hidden" unless empty? %>"> | ||
| <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
There was a problem hiding this comment. 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.
Comment on lines
+43
to
67
There was a problem hiding this comment. The |
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,6 @@ | ||||||||||||
| <ul | ||||||||||||
| data-mobile-menu-target="sidebar" | ||||||||||||
|
||||||||||||
| data-mobile-menu-target="sidebar" | |
| data-mobile-menu-target="sidebar" | |
| role="navigation" | |
| aria-label="Main navigation" | |
| aria-hidden="true" |
There was a problem hiding this comment.
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.
| 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() { | ||||||||
|
There was a problem hiding this comment. 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
|
||||||||
| 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() | ||||||||
| } | ||||||||
| } | ||||||||
| } | ||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,18 +1,20 @@ | ||
| <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 %> | ||
| <span class="hidden sm:inline"><%= t(".actions.edit") %></span> | ||
| <span class="sm:hidden text-xs whitespace-nowrap">Edit Journal</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 %> | ||
| <span class="hidden sm:inline"><%= t(".actions.delete") %></span> | ||
| <span class="sm:hidden text-xs whitespace-nowrap">Delete Journal</span> | ||
|
||
| <% end %> | ||
| </div> | ||
| </div> | ||
| <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 %> | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -24,10 +24,28 @@ | |||||||
| </head> | ||||||||
|
|
||||||||
| <body class="min-h-screen bg-base-100"> | ||||||||
| <div class="flex min-h-screen" data-controller="mobile-menu"> | ||||||||
|
||||||||
| <div class="flex min-h-screen" data-controller="mobile-menu"> | |
| <div class="flex min-h-screen" <%= 'data-controller="mobile-menu"' unless current_page?(login_path) %>> |
Outdated
There was a problem hiding this comment.
The hamburger menu button lacks proper accessibility attributes. It should include aria-label to describe its purpose (e.g., "Toggle navigation menu"), aria-expanded to indicate the menu state (true/false), and aria-controls to reference the sidebar element it controls. These attributes are essential for screen reader users to understand and operate the navigation menu.
There was a problem hiding this comment.
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.
| data-action="click->mobile-menu#close" | |
| data-action="click->mobile-menu#close" | |
| aria-hidden="true" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,28 +1,31 @@ | ||
| <section class="mx-4 sm:mx-8 my-6 sm:my-10"> | ||
| <div class="flex flex-col gap-4"> | ||
| <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-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 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 %> | ||
| <span class="hidden sm:inline"><%= t(".actions.edit") %></span> | ||
| <span class="sm:hidden text-xs whitespace-nowrap">Edit Project</span> | ||
| <% end %> | ||
|
|
||
| <%= render ActionButtonComponent.new(to: new_project_subproject_path(@project), icon: "add", size: :large, turbo_stream: true, colour: :primary) do %> | ||
| <span class="hidden sm:inline"><%= t(".create_subproject") %></span> | ||
| <span class="sm:hidden text-xs whitespace-nowrap">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 %> | ||
| <span class="hidden sm:inline"><%= t(".actions.delete") %></span> | ||
| <span class="sm:hidden text-xs whitespace-nowrap">Delete Project</span> | ||
|
||
| <% end %> | ||
| </div> | ||
| </div> | ||
|
|
||
| <%= render ContentCardComponent.new do %> | ||
| <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 %> | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,8 +1,9 @@ | ||||||
| <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 %> | ||||||
| <span class="hidden sm:inline"><%= t(".actions.create") %></span> | ||||||
| <span class="sm:hidden">New</span> | ||||||
|
There was a problem hiding this comment. Hardcoded text "New" should be internationalized. The full button text uses
Suggested change
|
||||||
| <% end %> | ||||||
| </div> | ||||||
| <%= render ContentCardComponent.new do %> | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The modal has both
mx-4on the inner div andp-4on the parent, which could result in excessive horizontal spacing on mobile devices (8px padding from parent + 16px horizontal margin from child = 24px total on each side). Themx-4class on the modal dialog is redundant since the parent already hasp-4padding that provides spacing from the viewport edges.