Fix/filtering UI #418

NavvneetK · 2025-11-26T13:10:59Z

This pull request refactors the faculty and staff filtering UI to improve code organization, reusability, and mobile user experience. The main changes include extracting filter components into reusable client-side components, introducing a new mobile filter panel, and updating the main page to use these components for both desktop and mobile layouts.

Component extraction and UI refactor:

Moved department and designation filter logic from page.tsx into new reusable components (DepartmentsClient, DesignationsClient, and MobileFilters) in client-components.tsx, simplifying and centralizing filter UI code. (app/[locale]/faculty-and-staff/client-components.tsxR4-R145, app/[locale]/faculty-and-staff/client-components.tsxR201-R363)
Replaced inline department filter implementation in page.tsx with the new DepartmentsClient component, removing duplicated logic and improving maintainability. (app/[locale]/faculty-and-staff/page.tsxL224-L298)

Mobile filter improvements:

Added a new MobileFilters component for mobile devices, providing a slide-in filter panel with designation and department filters, and handling scroll locking and outside click to close. (app/[locale]/faculty-and-staff/client-components.tsxR4-R145)
Updated the main faculty and staff page to use MobileFilters for mobile filter UI, replacing previous mobile-specific filter implementations. (app/[locale]/faculty-and-staff/page.tsxL76-L94)

General code and UI improvements:

Updated the import statements in page.tsx to use the new filter components. (app/[locale]/faculty-and-staff/page.tsxL23-R28)
Fetched department data for mobile filters in page.tsx to support the new mobile filter panel. (app/[locale]/faculty-and-staff/page.tsxR43-R46)
Improved layout and styling for filter sections, including scroll areas and responsive design adjustments. (app/[locale]/faculty-and-staff/page.tsxL51, app/[locale]/faculty-and-staff/page.tsxR76)

These changes make the filter UI more modular, maintainable, and user-friendly, especially for mobile users.

… larger screens

…ndStaff component

Copilot

Pull request overview

This pull request refactors the faculty and staff filtering UI by extracting filter logic into reusable client-side components and introducing a mobile-optimized filter panel. The changes improve code organization and enhance the mobile user experience.

Key Changes:

Extracted department and designation filter logic from page.tsx into new reusable client components (DepartmentsClient, DesignationsClient, MobileFilters)
Introduced a full-screen slide-in mobile filter panel with backdrop and scroll locking
Replaced inline mobile filter implementations with the new modular components

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.

File	Description
app/[locale]/faculty-and-staff/client-components.tsx	Added three new client components for mobile filters, department filtering, and designation filtering with improved UI features like "View more" functionality
app/[locale]/faculty-and-staff/page.tsx	Updated to use new client components, added department data fetching for mobile filters, and contains large blocks of commented-out code that should be removed

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

Copilot · 2025-11-26T13:15:40Z

app/[locale]/faculty-and-staff/client-components.tsx

+import { FaFilter, FaSlidersH, FaTimes, FaChevronLeft, FaChevronRight } from 'react-icons/fa';
+import {
+  Select,
+  SelectContent,
+  SelectItem,


Unused imports detected. The icons FaFilter, FaSlidersH, FaChevronLeft, and FaChevronRight are imported but never used in the component. Only FaTimes is actually used (line 114). Additionally, SelectItem is imported but never used. Remove these unused imports to improve code clarity and reduce bundle size.

Suggested change

import { FaFilter, FaSlidersH, FaTimes, FaChevronLeft, FaChevronRight } from 'react-icons/fa';

import {

Select,

SelectContent,

SelectItem,

import { FaTimes } from 'react-icons/fa';

import {

Select,

SelectContent,

Copilot · 2025-11-26T13:15:40Z

app/[locale]/faculty-and-staff/client-components.tsx

+        )}
+      >
+        <MdFilterList className="text-primary-700 text-xl"/>
+        <span className='text-primary-700'>Filters</span>


Inconsistent quote style. This line uses single quotes for the className string, while line 72 uses double quotes. For consistency with the rest of the file, use double quotes here as well.

Suggested change

<span className='text-primary-700'>Filters</span>

<span className="text-primary-700">Filters</span>

Copilot · 2025-11-26T13:15:41Z

app/[locale]/faculty-and-staff/client-components.tsx

+          <div >
+            <div className=" bg-white p-5">
+              <ScrollArea className="h-[calc(100vh-200px)]">
+                <div className="rounded-lg ">


Empty div wrapper without purpose. Line 103 contains an empty <div> tag that wraps the content but doesn't add any classes or attributes. This adds unnecessary nesting to the DOM. Consider removing it or adding necessary attributes if it serves a purpose.

Suggested change

<div >

<div className=" bg-white p-5">

<ScrollArea className="h-[calc(100vh-200px)]">

<div className="rounded-lg ">

<div className=" bg-white p-5">

<ScrollArea className="h-[calc(100vh-200px)]">

<div className="rounded-lg ">

Copilot · 2025-11-26T13:15:41Z

app/[locale]/faculty-and-staff/client-components.tsx

+      <ol className="w-full space-y-4">
+        {/* Constrain the list height and allow internal scrolling */}
+        <div
+          className={cn(
+            'overflow-y-auto',
+            showAll ? 'max-h-[calc(100vh-200px)]' : 'max-h-[280px]'
+          )}
+        >
+          {visible.map(({ name, urlName }, index) => (
+            <li key={index}>
+              <PreserveParamsLink
+                paramToUpdate="department"
+                value={getUpdatedDepartments(urlName)}
+                className={cn(
+                  'flex w-full items-center rounded border p-3',
+                  selectedDepartments.includes(urlName)
+                    ? 'bg-primary-50 border-primary-700'
+                    : 'border-neutral-300'
+                )}
+              >
+                <div className="flex w-full items-center">
+                  <div className="mr-2">
+                    <input
+                      type="checkbox"
+                      id={`department-${urlName}`}
+                      className="h-4 w-4 rounded border-neutral-300 text-primary-700 focus:ring-primary-700"
+                      checked={selectedDepartments.includes(urlName)}
+                      readOnly
+                    />
+                  </div>
+                  <span className="font-semibold text-shade-dark">{name}</span>
+                </div>
+              </PreserveParamsLink>
+            </li>
+          ))}
+        </div>


Incorrect semantic structure. The <ol> element on line 272 wraps a <div> on line 274, but the list items (<li>) are inside the div. This creates invalid HTML since a <div> cannot be a direct child of <ol>. The div should be removed and the overflow/max-height styles should be applied directly to the <ol> element, or the structure should be reorganized.

Suggested change

<ol className="w-full space-y-4">

{/* Constrain the list height and allow internal scrolling */}

<div

className={cn(

'overflow-y-auto',

showAll ? 'max-h-[calc(100vh-200px)]' : 'max-h-[280px]'

)}

>

{visible.map(({ name, urlName }, index) => (

<li key={index}>

<PreserveParamsLink

paramToUpdate="department"

value={getUpdatedDepartments(urlName)}

className={cn(

'flex w-full items-center rounded border p-3',

selectedDepartments.includes(urlName)

? 'bg-primary-50 border-primary-700'

: 'border-neutral-300'

)}

>

<div className="flex w-full items-center">

<div className="mr-2">

<input

type="checkbox"

id={`department-${urlName}`}

className="h-4 w-4 rounded border-neutral-300 text-primary-700 focus:ring-primary-700"

checked={selectedDepartments.includes(urlName)}

readOnly

/>

</div>

<span className="font-semibold text-shade-dark">{name}</span>

</div>

</PreserveParamsLink>

</li>

))}

</div>

<ol

className={cn(

'w-full space-y-4 overflow-y-auto',

showAll ? 'max-h-[calc(100vh-200px)]' : 'max-h-[280px]'

)}

>

{/* Constrain the list height and allow internal scrolling */}

{visible.map(({ name, urlName }, index) => (

<li key={index}>

<PreserveParamsLink

paramToUpdate="department"

value={getUpdatedDepartments(urlName)}

className={cn(

'flex w-full items-center rounded border p-3',

selectedDepartments.includes(urlName)

? 'bg-primary-50 border-primary-700'

: 'border-neutral-300'

)}

>

<div className="flex w-full items-center">

<div className="mr-2">

<input

type="checkbox"

id={`department-${urlName}`}

className="h-4 w-4 rounded border-neutral-300 text-primary-700 focus:ring-primary-700"

checked={selectedDepartments.includes(urlName)}

readOnly

/>

</div>

<span className="font-semibold text-shade-dark">{name}</span>

</div>

</PreserveParamsLink>

</li>

))}

Copilot · 2025-11-26T13:15:42Z

app/[locale]/faculty-and-staff/client-components.tsx

+                    <button
+                      onClick={() => setOpen(false)}
+                      aria-label="Close filters"
+                      className=" hover:bg-black/5"
+                    >
+                      <FaTimes className="text-primary-700 size-7" />
+                    </button>


Missing accessibility label. The close button for the mobile filter panel should have an aria-label attribute to provide a descriptive label for screen readers. While line 111 has aria-label="Close filters", this button is missing additional context. Consider adding title="Close filters" as well for tooltip support.

Copilot · 2025-11-26T13:15:42Z

app/[locale]/faculty-and-staff/client-components.tsx

+        aria-controls="mobile-filters-panel"
+        onClick={() => setOpen((s) => !s)}
+        className={cn(
+          'flex items-center gap-2 px-3 py-2 rounded border border-primary-100 bg-neutral-50 ',


Missing spacing in className. There's a trailing space in the className string that should be removed. Change 'flex items-center gap-2 px-3 py-2 rounded border border-primary-100 bg-neutral-50 ' to 'flex items-center gap-2 px-3 py-2 rounded border border-primary-100 bg-neutral-50'.

Suggested change

'flex items-center gap-2 px-3 py-2 rounded border border-primary-100 bg-neutral-50 ',

'flex items-center gap-2 px-3 py-2 rounded border border-primary-100 bg-neutral-50',

Copilot · 2025-11-26T13:15:42Z

app/[locale]/faculty-and-staff/client-components.tsx

+  SelectValue,
+} from '~/components/inputs';
+import { cn } from '~/lib/utils';
+import { MdFilterList } from "react-icons/md";


Inconsistent quote style. This line uses double quotes for the import, while most other imports in the file use single quotes. For consistency with the rest of the file (lines 1-15), use single quotes here as well.

Suggested change

import { MdFilterList } from "react-icons/md";

import { MdFilterList } from 'react-icons/md';

Copilot · 2025-11-26T13:15:43Z

app/[locale]/faculty-and-staff/page.tsx

+  // const selectedDepartments = Array.isArray(department)
+  //   ? department
+  //   : department
+  //     ? [department]
+  //     : [];

-  // Define the updated department value based on selection
-  const getUpdatedDepartments = (urlName: string) => {
-    return selectedDepartments.includes(urlName)
-      ? selectedDepartments.filter((d) => d !== urlName)
-      : [...selectedDepartments, urlName];
-  };
+  // // Define the updated department value based on selection
+  // const getUpdatedDepartments = (urlName: string) => {
+  //   return selectedDepartments.includes(urlName)
+  //     ? selectedDepartments.filter((d) => d !== urlName)
+  //     : [...selectedDepartments, urlName];
+  // };

-  return select ? (
-    <Select navigate>
-      <SelectTrigger className="px-4 py-5 sm:w-1/2 lg:w-1/3 xl:hidden">
-        <SelectValue
-          placeholder={
-            selectedDepartments.length
-              ? `${selectedDepartments.length} selected`
-              : 'Choose a department'
-          }
-        />
-      </SelectTrigger>
-      <SelectContent>
-        {departments.map(({ name, urlName }, index) => (
-          <div key={index} className="flex items-center px-2 py-1">
-            <input
-              type="checkbox"
-              id={`mobile-department-${urlName}`}
-              className="h-4 w-4 rounded border-neutral-300 text-primary-700 focus:ring-primary-700"
-              checked={selectedDepartments.includes(urlName)}
-              readOnly
-            />
-            <PreserveParamsLink
-              paramToUpdate="department"
-              value={getUpdatedDepartments(urlName)}
-              className="ml-2 w-full py-1"
-            >
-              {name}
-            </PreserveParamsLink>
-          </div>
-        ))}
-      </SelectContent>
-    </Select>
-  ) : (
-    <ol className="w-full space-y-4">
-      {departments.map(({ name, urlName }, index) => (
-        <li key={index}>
-          <PreserveParamsLink
-            paramToUpdate="department"
-            value={getUpdatedDepartments(urlName)}
-            className={cn(
-              'flex w-full items-center rounded border p-3',
-              selectedDepartments.includes(urlName)
-                ? 'bg-primary-50 border-primary-700'
-                : 'border-neutral-300'
-            )}
-          >
-            <div className="flex w-full items-center">
-              <div className="mr-2">
-                <input
-                  type="checkbox"
-                  id={`department-${urlName}`}
-                  className="h-4 w-4 rounded border-neutral-300 text-primary-700 focus:ring-primary-700"
-                  checked={selectedDepartments.includes(urlName)}
-                  readOnly
-                />
-              </div>
-              <span className="font-semibold text-shade-dark">{name}</span>
-            </div>
-          </PreserveParamsLink>
-        </li>
-      ))}
-    </ol>
+  // return select ? (
+  //   <Select navigate>
+  //     <SelectTrigger className="px-4 py-5 sm:w-1/2 lg:w-1/3 xl:hidden">
+  //       <SelectValue
+  //         placeholder={
+  //           selectedDepartments.length
+  //             ? `${selectedDepartments.length} selected`
+  //             : 'Choose a department'
+  //         }
+  //       />
+  //     </SelectTrigger>
+  //     <SelectContent>
+  //       {departments.map(({ name, urlName }, index) => (
+  //         <div key={index} className="flex items-center px-2 py-1">
+  //           <input
+  //             type="checkbox"
+  //             id={`mobile-department-${urlName}`}
+  //             className="h-4 w-4 rounded border-neutral-300 text-primary-700 focus:ring-primary-700"
+  //             checked={selectedDepartments.includes(urlName)}
+  //             readOnly
+  //           />
+  //           <PreserveParamsLink
+  //             paramToUpdate="department"
+  //             value={getUpdatedDepartments(urlName)}
+  //             className="ml-2 w-full py-1"
+  //           >
+  //             {name}
+  //           </PreserveParamsLink>
+  //         </div>
+  //       ))}
+  //     </SelectContent>
+  //   </Select>
+  // ) : (
+  //   <ol className="w-full space-y-4">
+  //     {departments.map(({ name, urlName }, index) => (
+  //       <li key={index}>
+  //         <PreserveParamsLink
+  //           paramToUpdate="department"
+  //           value={getUpdatedDepartments(urlName)}
+  //           className={cn(
+  //             'flex w-full items-center rounded border p-3',
+  //             selectedDepartments.includes(urlName)
+  //               ? 'bg-primary-50 border-primary-700'
+  //               : 'border-neutral-300'
+  //           )}
+  //         >
+  //           <div className="flex w-full items-center">
+  //             <div className="mr-2">
+  //               <input
+  //                 type="checkbox"
+  //                 id={`department-${urlName}`}
+  //                 className="h-4 w-4 rounded border-neutral-300 text-primary-700 focus:ring-primary-700"
+  //                 checked={selectedDepartments.includes(urlName)}
+  //                 readOnly
+  //               />
+  //             </div>
+  //             <span className="font-semibold text-shade-dark">{name}</span>
+  //           </div>
+  //         </PreserveParamsLink>
+  //       </li>
+  //     ))}
+  //   </ol>
+  // );


Large block of commented-out code. Lines 231-306 contain a significant amount of commented-out code that has been moved to the client component. This dead code should be removed to improve code maintainability and clarity. If this code needs to be preserved for reference, it should be documented in version control history instead.

Copilot · 2025-11-26T13:15:43Z

app/[locale]/faculty-and-staff/client-components.tsx

+interface Dept {
+  id: number;
+  name: string;
+  urlName: string;
+}
+


Duplicate interface definition. The Dept interface is defined twice in this file (lines 20-24 and lines 202-206). This duplication can lead to maintenance issues if the interface needs to be updated. Consider defining it once at the top of the file and reusing it throughout.

Suggested change

interface Dept {

id: number;

name: string;

urlName: string;

}

NavvneetK added 2 commits November 20, 2025 12:51

fix:implement DepartmentsClient component for department filtering on…

4072dc4

… larger screens

feat: add mobile filters for departments and designations in FacultyA…

3893bd0

…ndStaff component

NavvneetK requested a review from Copilot November 26, 2025 13:10

Copilot started reviewing on behalf of NavvneetK November 26, 2025 13:11 View session

Copilot finished reviewing on behalf of NavvneetK November 26, 2025 13:13

Copilot AI reviewed Nov 26, 2025

View reviewed changes

Debatreya changed the base branch from master to staged November 26, 2025 21:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix/filtering UI #418

Fix/filtering UI #418

NavvneetK commented Nov 26, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 26, 2025

Uh oh!

Copilot AI Nov 26, 2025

Uh oh!

Copilot AI Nov 26, 2025

Uh oh!

Copilot AI Nov 26, 2025

Uh oh!

Copilot AI Nov 26, 2025

Uh oh!

Copilot AI Nov 26, 2025

Uh oh!

Copilot AI Nov 26, 2025

Uh oh!

Copilot AI Nov 26, 2025

Uh oh!

Copilot AI Nov 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	<span className='text-primary-700'>Filters</span>
	<span className="text-primary-700">Filters</span>

	'flex items-center gap-2 px-3 py-2 rounded border border-primary-100 bg-neutral-50 ',
	'flex items-center gap-2 px-3 py-2 rounded border border-primary-100 bg-neutral-50',

	import { MdFilterList } from "react-icons/md";
	import { MdFilterList } from 'react-icons/md';

Fix/filtering UI #418

Are you sure you want to change the base?

Fix/filtering UI #418

Conversation

NavvneetK commented Nov 26, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants