Skip to content

Conversation

@CulmoneY
Copy link
Contributor

TL;DR

Briefly describe what this PR does and why it is needed. Include any relevant background or context.

Added a universal sidebar over navbar to adhear to designs

image

Checklist

  • Changes have been top-hatted locally
  • Tests have been added or updated
  • Documentation has been updated (if applicable)
  • Linked related issues

@CulmoneY CulmoneY requested review from ch-iv, Copilot and pranavrao145 and removed request for Copilot December 29, 2025 01:47
Copilot AI review requested due to automatic review settings December 29, 2025 01:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR replaces the existing navbar component with a new sidebar navigation component to align with design specifications. The sidebar provides navigation for admin functions, projects, and user logout functionality.

  • Replaced horizontal navbar with a vertical sidebar component
  • Added new icon assets for sidebar navigation items
  • Restructured the layout to use a flex container with sidebar and main content area

Reviewed changes

Copilot reviewed 5 out of 11 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
app/views/layouts/application.html.erb Updated layout to render sidebar instead of navbar, wrapped content in flex container
app/components/sidebar_component.rb New component class that builds admin and projects navigation sections
app/components/sidebar_component.html.erb New sidebar template with navigation menu, logo, and logout button
app/components/navbar_component.rb Removed old navbar component class
app/components/navbar_component.html.erb Removed old navbar template
app/assets/images/icons/*.svg Added new icon assets for sidebar navigation (person-gear, compass, folder, box-arrow-left)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@@ -0,0 +1,36 @@
<ul class="menu shadow-lg bg-base-200 w-65 min-h-screen tracking-wide justify-between">
Copy link

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

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

The class w-65 is not a standard Tailwind CSS class. Tailwind's default width utilities use increments like w-64 (16rem) or w-72 (18rem). If this is intended to be a custom width, ensure it's defined in your Tailwind configuration. Otherwise, this should likely be w-64 or another standard width class.

Suggested change
<ul class="menu shadow-lg bg-base-200 w-65 min-h-screen tracking-wide justify-between">
<ul class="menu shadow-lg bg-base-200 w-64 min-h-screen tracking-wide justify-between">

Copilot uses AI. Check for mistakes.
Comment on lines +2 to +35
<div>
<div class="px-4 py-4 border-b border-base-300">
<%= link_to root_path do %>
<div class="flex items-center gap-3">
<%= render IconComponent.new(icon: "tree", size: :md) %>
<span class="font-semibold text-sm text-base-content">
INGA INTERNALS
</span>
</div>
<% end %>
</div>
<li class="py-6">
<% sections.each do |section| %>
<h2 class="menu-title uppercase"> <%= section.title %></h2>
<ul>
<% section.items.each do |item| %>
<li> <%= link_to item.path do %>
<%= render IconComponent.new(icon: item.icon, size: :md, classes: "w-5 h-5") %>
<span class="truncate text-base-content"><%= item.name %></span>
<% end %>
</li>
<% end %>
</ul>
<% end %>
</li>
</div>
<div class="border-t border-base-300 py-2">
<li>
<%= button_to logout_path, method: :delete, class: "flex gap-3" do %>
<%= render IconComponent.new(icon: "box-arrow-left", size: :md, classes: "w-5 h-5") %>
<span>Logout</span>
<% end %>
</li>
</div>
Copy link

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

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

Invalid HTML structure: Lines 2, 28, and 35 contain <div> elements as direct children of the <ul> element, which violates HTML standards. A <ul> element should only contain <li> elements as direct children. Consider restructuring the HTML by either wrapping the divs in <li> elements or changing the root element to a semantic element like <nav> or <aside>.

Copilot uses AI. Check for mistakes.
Comment on lines 1 to 36
<ul class="menu shadow-lg bg-base-200 w-65 min-h-screen tracking-wide justify-between">
<div>
<div class="px-4 py-4 border-b border-base-300">
<%= link_to root_path do %>
<div class="flex items-center gap-3">
<%= render IconComponent.new(icon: "tree", size: :md) %>
<span class="font-semibold text-sm text-base-content">
INGA INTERNALS
</span>
</div>
<% end %>
</div>
<li class="py-6">
<% sections.each do |section| %>
<h2 class="menu-title uppercase"> <%= section.title %></h2>
<ul>
<% section.items.each do |item| %>
<li> <%= link_to item.path do %>
<%= render IconComponent.new(icon: item.icon, size: :md, classes: "w-5 h-5") %>
<span class="truncate text-base-content"><%= item.name %></span>
<% end %>
</li>
<% end %>
</ul>
<% end %>
</li>
</div>
<div class="border-t border-base-300 py-2">
<li>
<%= button_to logout_path, method: :delete, class: "flex gap-3" do %>
<%= render IconComponent.new(icon: "box-arrow-left", size: :md, classes: "w-5 h-5") %>
<span>Logout</span>
<% end %>
</li>
</div>
</ul>
Copy link

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

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

The sidebar lacks semantic HTML structure and ARIA attributes for accessibility. Consider wrapping the sidebar in a <nav> element with aria-label="Main navigation" or <aside> with an appropriate role. Additionally, the logout button could benefit from an aria-label to clarify its purpose for screen reader users.

Suggested change
<ul class="menu shadow-lg bg-base-200 w-65 min-h-screen tracking-wide justify-between">
<div>
<div class="px-4 py-4 border-b border-base-300">
<%= link_to root_path do %>
<div class="flex items-center gap-3">
<%= render IconComponent.new(icon: "tree", size: :md) %>
<span class="font-semibold text-sm text-base-content">
INGA INTERNALS
</span>
</div>
<% end %>
</div>
<li class="py-6">
<% sections.each do |section| %>
<h2 class="menu-title uppercase"> <%= section.title %></h2>
<ul>
<% section.items.each do |item| %>
<li> <%= link_to item.path do %>
<%= render IconComponent.new(icon: item.icon, size: :md, classes: "w-5 h-5") %>
<span class="truncate text-base-content"><%= item.name %></span>
<% end %>
</li>
<% end %>
</ul>
<% end %>
</li>
</div>
<div class="border-t border-base-300 py-2">
<li>
<%= button_to logout_path, method: :delete, class: "flex gap-3" do %>
<%= render IconComponent.new(icon: "box-arrow-left", size: :md, classes: "w-5 h-5") %>
<span>Logout</span>
<% end %>
</li>
</div>
</ul>
<nav aria-label="Main navigation">
<ul class="menu shadow-lg bg-base-200 w-65 min-h-screen tracking-wide justify-between">
<div>
<div class="px-4 py-4 border-b border-base-300">
<%= link_to root_path do %>
<div class="flex items-center gap-3">
<%= render IconComponent.new(icon: "tree", size: :md) %>
<span class="font-semibold text-sm text-base-content">
INGA INTERNALS
</span>
</div>
<% end %>
</div>
<li class="py-6">
<% sections.each do |section| %>
<h2 class="menu-title uppercase"> <%= section.title %></h2>
<ul>
<% section.items.each do |item| %>
<li> <%= link_to item.path do %>
<%= render IconComponent.new(icon: item.icon, size: :md, classes: "w-5 h-5") %>
<span class="truncate text-base-content"><%= item.name %></span>
<% end %>
</li>
<% end %>
</ul>
<% end %>
</li>
</div>
<div class="border-t border-base-300 py-2">
<li>
<%= button_to logout_path, method: :delete, class: "flex gap-3", aria: { label: "Log out of application" } do %>
<%= render IconComponent.new(icon: "box-arrow-left", size: :md, classes: "w-5 h-5") %>
<span>Logout</span>
<% end %>
</li>
</div>
</ul>
</nav>

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@ch-iv ch-iv left a comment

Choose a reason for hiding this comment

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

lgtm

@CulmoneY CulmoneY merged commit be28d54 into main Jan 5, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants