Skip to content

Feat/payslip frontend#25

Draft
saknarajapakshe wants to merge 5 commits intoLSFLK:mainfrom
saknarajapakshe:feat/payslip-frontend
Draft

Feat/payslip frontend#25
saknarajapakshe wants to merge 5 commits intoLSFLK:mainfrom
saknarajapakshe:feat/payslip-frontend

Conversation

@saknarajapakshe
Copy link
Copy Markdown
Contributor

#22

🛠 Frontend – Full Pay‑Slip Implementation

This branch delivers the complete React/TypeScript micro‑app for the pay‑slip feature, including
authentication, user/admin workflows, UI components, and native‑bridge integration.
Every screen, hook and utility required by the mobile SuperApp has been built out.

🎯 Highlights

  • Authentication & bridge

    • useAuth hook handles token retrieval, profile fetching and admin detection.
    • useBridge encapsulates native‑bridge token requests with retry logic.
  • Data hooks

    • usePaySlips fetches pay‑slips; supports both user and admin views, with optional user‑scoped queries.
    • useUsers returns the user list (admin‑only) with loading/error handling.
  • Views

    • User‑facing: PaySlipList + PaySlipDetail with filters, in‑app PDF viewer, external download.
    • Admin‑facing:
      • AdminPaySlipList – global pay‑slip list with refresh.
      • AllUsersView – user directory with loading/error/empty states.
      • AdminUserDetailView – per‑user slips, upload & delete actions.
  • UI & utilities

    • Reusable components: Filters, UploadModal, LoadingState, EmptyState, ErrorState, AppPickerModal, etc.
    • File‑download helpers (native vs. web blob fallback).
    • Date formatting and constants centralized.
    • Tailwind CSS for responsive, mobile‑first styling; custom modal animations.
  • Infrastructure

    • Vite config with env handling and proxy support.
    • Tailwind config and global styles.
    • Mock server (mock-server/server.cjs) providing tokens and sample data for dev.
    • Types defined in src/types.

✅ Features Implemented

  • Authentication through SuperApp bridge.
  • Pay‑slip listing, filtering (month/year), sorting (newest first).
  • PDF viewing: in‑app viewer + open‑in‑external‑app/download.
  • Admin upload flow:
    1. Validate PDF (type/size).
    2. Check for duplicates (month/year).
    3. Two‑step backend call (/api/upload then /api/pay-slips).
  • Admin delete with confirmation.
  • Global refresh and navigation between users/pay‑slips.
  • Error/loading/empty states across all screens.
  • Native download support with fallback.
  • Configuration and constants (upload params, date ranges, API retry settings).

📁 Changeset Summary

  • Added entire pay-slip-app/frontend directory: sources, hooks, views, components, assets, configs.
  • Created utility modules, constants, and types.
  • Updated build & config files (vite.config.ts, tailwind.config.cjs, etc.).
  • Added README documenting setup, features, mock tokens, and API endpoints.

Note: initial scaffolding (project initialization) was merged in a separate PR; this one focuses on
the full frontend implementation and feature completeness.

🚀 Getting Started

  1. cd pay-slip-app/frontend && npm install
  2. Copy .env.example.env, set VITE_API_BASE/VITE_DEV_TOKEN.
  3. Run with mock backend: npm run dev and npm run mock
  4. Use admin-token / user-token etc. to exercise admin and user journeys.
  5. Build with npm run build for production output (dist/index.html).

@gemini-code-assist
Copy link
Copy Markdown

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces the complete frontend micro-application for the pay-slip feature, enabling both employees and administrators to manage and view pay slips efficiently. It establishes a robust foundation with comprehensive authentication, data fetching, and UI components tailored for a mobile SuperApp environment, ensuring a seamless user experience and streamlined administrative tasks.

Highlights

  • Complete Frontend Implementation: Delivered the full React/TypeScript micro-app for the pay-slip feature, encompassing authentication, user and admin workflows, UI components, and native-bridge integration.
  • Authentication & Bridge Integration: Implemented useAuth for token retrieval, profile fetching, and admin detection, and useBridge for native-bridge token requests with retry logic.
  • Data Management Hooks: Introduced usePaySlips for fetching pay-slips (supporting user and admin views with optional user-scoped queries) and useUsers for admin-only user list retrieval.
  • User & Admin Views: Provided user-facing PaySlipList and PaySlipDetail with filters, in-app PDF viewer, and external download options. Admin-facing views include AdminPaySlipList, AllUsersView, and AdminUserDetailView for managing user slips, uploads, and deletions.
  • UI & Utilities: Developed reusable UI components (e.g., Filters, UploadModal, LoadingState, EmptyState, ErrorState), file-download helpers, date formatting, and centralized constants, all styled with Tailwind CSS.
  • Infrastructure & Mocking: Configured Vite for environment handling and proxy support, set up Tailwind CSS, and included a mock server (mock-server/server.cjs) for development and testing with sample data and token simulation.
  • Two-Step Admin Upload Flow: Implemented a robust admin upload process that first uploads the file to the backend via /api/upload to get a fileUrl, then creates the pay slip record with metadata via /api/pay-slips.
Changelog
  • leave-app/frontend/package-lock.json
    • Removed 'peer': true property from several dependency entries.
  • pay-slip-app/backend/api/openapi.yaml
    • Added OpenAPI specification for the Pay-Slip-App API, detailing endpoints for health checks, user management, pay slip operations, and file uploads.
  • pay-slip-app/frontend/.env.example
    • Added an example environment configuration file for the frontend, including VITE_DEV_TOKEN, VITE_API_BASE, and VITE_PROXY_TARGET.
  • pay-slip-app/frontend/.gitignore
    • Added a .gitignore file for the frontend, excluding common build artifacts, node modules, environment files, and IDE configurations.
  • pay-slip-app/frontend/README.md
    • Added a comprehensive README file detailing the frontend's features for users and admins, technology stack, project structure, setup and development instructions, configuration, API endpoints, and implementation details.
  • pay-slip-app/frontend/index.html
    • Added the main HTML entry point for the React application, including viewport meta tags, Tailwind CSS CDN, and custom styling.
  • pay-slip-app/frontend/microapp.json
    • Added microapp metadata, including name, appId, version, description, and clientId.
  • pay-slip-app/frontend/mock-server/server.cjs
    • Added a mock Express.js server to simulate backend API endpoints for users and pay slips, supporting authentication, CRUD operations, and mock data.
  • pay-slip-app/frontend/package.json
    • Added package.json for the pay-slip-app frontend, defining project metadata, scripts, dependencies (e.g., React, clsx, lucide-react), and devDependencies (e.g., Vite, Tailwind CSS, TypeScript, Express).
  • pay-slip-app/frontend/postcss.config.cjs
    • Added PostCSS configuration to integrate Tailwind CSS and Autoprefixer.
  • pay-slip-app/frontend/scripts/sync-microapp-version.cjs
    • Added a Node.js script to synchronize the version number between package.json and microapp.json.
  • pay-slip-app/frontend/src/App.tsx
    • Added the main React application component, managing authentication, user/admin views, navigation, and the two-step pay slip upload process.
  • pay-slip-app/frontend/src/api/client.ts
    • Added an API client with fetchWithRetry logic for robust network requests and functions for interacting with user and pay slip endpoints.
  • pay-slip-app/frontend/src/bridge.ts
    • Added native bridge integration logic, providing functions to get and save local data, and detect if running within a WebView.
  • pay-slip-app/frontend/src/components/AppPickerModal.tsx
    • Added a modal component allowing users to choose between viewing a PDF in-app or opening it with an external application.
  • pay-slip-app/frontend/src/components/EmptyState.tsx
    • Added a reusable React component for displaying an empty state message with an icon.
  • pay-slip-app/frontend/src/components/ErrorState.tsx
    • Added a reusable React component for displaying error messages with an optional retry button.
  • pay-slip-app/frontend/src/components/Filters.tsx
    • Added a filter component for selecting month and year, with expand/collapse functionality and a clear all option.
  • pay-slip-app/frontend/src/components/LoadingState.tsx
    • Added a reusable React component for displaying a loading animation with placeholder cards.
  • pay-slip-app/frontend/src/components/PDFViewer.tsx
    • Added an in-app PDF viewer component using an iframe, with loading and error handling.
  • pay-slip-app/frontend/src/components/PaySlipCard.tsx
    • Added a component to display individual pay slip information, including view and download actions, integrating with the native bridge for downloads.
  • pay-slip-app/frontend/src/components/UI.tsx
    • Added a collection of reusable UI components including Button, Card, Badge, Input, Select, and Modal with consistent styling and functionality.
  • pay-slip-app/frontend/src/components/UploadModal.tsx
    • Added a modal component for administrators to upload pay slips, including user selection, month/year input, file validation, and duplicate checking.
  • pay-slip-app/frontend/src/components/UserCard.tsx
    • Added a component to display individual user information with an avatar, email, and a navigation indicator.
  • pay-slip-app/frontend/src/constants.ts
    • Added a file centralizing application-wide constants for API configuration, bridge settings, upload limits, and date formatting.
  • pay-slip-app/frontend/src/hooks/useAuth.ts
    • Added a React hook (useAuth) to manage user authentication state, token retrieval from the bridge, and user profile fetching.
  • pay-slip-app/frontend/src/hooks/useBridge.ts
    • Added a React hook (useBridge) to handle native bridge interactions, including token requests with retry logic and file download requests.
  • pay-slip-app/frontend/src/hooks/usePaySlips.ts
    • Added a React hook (usePaySlips) for fetching and managing pay slip data, supporting both general and user-specific queries.
  • pay-slip-app/frontend/src/hooks/useUsers.ts
    • Added a React hook (useUsers) for fetching and managing user data, specifically for admin views.
  • pay-slip-app/frontend/src/index.tsx
    • Added the entry point for the React application, rendering the App component within a StrictMode.
  • pay-slip-app/frontend/src/types/index.ts
    • Added TypeScript type definitions for Role, User, PaySlip, PaySlipsResponse, and PaySlipsFilters.
  • pay-slip-app/frontend/src/utils/cn.ts
    • Added a utility function (cn) for conditionally joining CSS class names using clsx.
  • pay-slip-app/frontend/src/utils/downloadUtils.ts
    • Added utility functions (downloadFile, openFileInNewTab) for handling file downloads and opening files in new tabs, with CORS considerations.
  • pay-slip-app/frontend/src/utils/formatters.ts
    • Added utility functions for formatting dates, month-year combinations, and retrieving month names.
  • pay-slip-app/frontend/src/views/AdminPaySlipList.tsx
    • Added the AdminPaySlipList view, displaying all pay slips grouped by user, with filtering, summary statistics, and options to view PDFs.
  • pay-slip-app/frontend/src/views/AdminUserDetailView.tsx
    • Added the AdminUserDetailView, allowing administrators to view and manage pay slips for a specific user, including upload and delete functionalities.
  • pay-slip-app/frontend/src/views/AllUsersView.tsx
    • Added the AllUsersView, displaying a list of all users for administrators, with loading, empty, and error states.
  • pay-slip-app/frontend/src/views/PaySlipDetail.tsx
    • Added the PaySlipDetail view, showing detailed information for a single pay slip with options to download.
  • pay-slip-app/frontend/src/views/PaySlipList.tsx
    • Added the PaySlipList view for regular users, displaying their pay slips with filtering and options to view PDFs.
  • pay-slip-app/frontend/styles.css
    • Added global CSS styles, including basic HTML/body/root styling, font definitions, Tailwind CSS imports, and custom keyframe animations for modals.
  • pay-slip-app/frontend/tailwind.config.cjs
    • Added Tailwind CSS configuration, specifying content paths and extending the default theme with custom font families and primary colors.
  • pay-slip-app/frontend/tsconfig.json
    • Added TypeScript configuration for the frontend, defining compiler options for ES2020 target, JSX support, strict type checking, and module resolution.
  • pay-slip-app/frontend/vite.config.ts
    • Added Vite configuration for the frontend, enabling React plugin, single-file build, and proxy setup for API requests during development.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a comprehensive frontend implementation for the pay-slip micro-app. A critical finding is a potential Cross-Site Scripting (XSS) vulnerability, as pay slip URLs from the backend API are rendered in an iframe or used for downloads without sufficient validation, posing a risk despite administrative privilege requirements. Furthermore, a critical performance issue related to client-side data filtering needs addressing. There are also opportunities to enhance maintainability by removing dead code, reducing duplication, and ensuring UI component consistency, along with a potential API design issue regarding data handling on deletion that could lead to orphaned files.

Comment thread pay-slip-app/frontend/src/api/client.ts Outdated
Comment on lines +130 to +143
getPayslipsForUser: async (
token: string,
userId: string,
): Promise<PaySlipsResponse> => {
const all = await request<PaySlipsResponse>("/pay-slips", token);
const filtered = (all.data || []).filter(
(payslip) => String(payslip.userId) === String(userId),
);
return {
data: filtered,
total: filtered.length,
nextCursor: undefined,
};
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

The getPayslipsForUser function fetches all payslips and then filters them by userId on the client. This is highly inefficient and will not scale as the number of payslips grows, leading to poor performance and excessive data transfer. The backend API should be updated to support filtering by user ID (e.g., GET /api/users/{userId}/pay-slips or GET /api/pay-slips?userId={id}). The mock server already implements a similar endpoint, which suggests this was the intended design.

  getPayslipsForUser: async (
    token: string,
    userId: string,
  ): Promise<PaySlipsResponse> => {
    // This should call a dedicated backend endpoint for efficiency.
    // For example: GET /api/users/{userId}/pay-slips
    return request<PaySlipsResponse>(`/users/${userId}/pay-slips`, token);
  },

Comment thread pay-slip-app/backend/api/openapi.yaml Outdated
Comment thread pay-slip-app/frontend/src/App.tsx Outdated
Comment on lines +65 to +68
userEmail:
type: string
format: email
example: "alice@example.com"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The PaySlip schema includes userEmail, which is redundant given that userId is already present. This denormalization can lead to data inconsistency if a user's email changes. It's better to fetch the user's details via their userId when needed to ensure the email is always up-to-date.

Comment thread pay-slip-app/frontend/index.html Outdated
Comment thread pay-slip-app/frontend/src/bridge.ts Outdated
Comment thread pay-slip-app/frontend/src/components/UI.tsx Outdated
Comment thread pay-slip-app/frontend/src/components/UI.tsx
Comment thread pay-slip-app/frontend/src/components/UploadModal.tsx Outdated
Comment thread pay-slip-app/frontend/src/views/PaySlipDetail.tsx Outdated
@sthanikan2000
Copy link
Copy Markdown
Collaborator

/gemini review

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request delivers a complete and well-structured frontend application for the pay-slip feature. A critical security concern has been identified: potential Stored Cross-Site Scripting (XSS) vulnerabilities. Specifically, the fileUrl retrieved from the API is used in sensitive sinks (iframe src and anchor href) without proper protocol validation, which could pose a significant risk if an admin account is compromised. Strict protocol validation for all URLs in these contexts is recommended. Furthermore, consider enhancing type safety for the native bridge integration, improving code consistency, and addressing a minor documentation discrepancy.

Comment thread pay-slip-app/frontend/src/types/global.d.ts
Comment thread pay-slip-app/frontend/src/components/PDFViewer.tsx
Comment thread pay-slip-app/frontend/src/utils/downloadUtils.ts
Comment thread pay-slip-app/frontend/README.md Outdated
Comment thread pay-slip-app/frontend/src/App.tsx Outdated
Comment thread pay-slip-app/frontend/src/views/AdminUserDetailView.tsx Outdated
@saknarajapakshe saknarajapakshe force-pushed the feat/payslip-frontend branch 4 times, most recently from 1f6c3cf to 5c3149a Compare March 6, 2026 10:32

type View = "list" | "admin-users" | "admin-user-detail";

const App: React.FC = () => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this is doing alot: managing authentication state, several data fetching hooks like useAuth, usePaySlips, useUsers; and modal/deletion states

suggest: move out the deletion logic to a custom hook or manage via a reducer to keep this main component clean

@@ -0,0 +1,142 @@
import React from "react";
Copy link
Copy Markdown

@ginaxu1 ginaxu1 Mar 6, 2026

Choose a reason for hiding this comment

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

this UI.tsx nearly identical to the one in Leave app (duplicating core components like Button, Modal, and Input). should we write shared one for both to use?

Comment thread pay-slip-app/frontend/src/utils/cn.ts Outdated
Comment thread pay-slip-app/frontend/src/api/client.ts Outdated
Comment thread pay-slip-app/frontend/src/hooks/useBridge.ts Outdated
@saknarajapakshe saknarajapakshe force-pushed the feat/payslip-frontend branch 2 times, most recently from a04396e to b202371 Compare March 9, 2026 06:59
@saknarajapakshe saknarajapakshe marked this pull request as ready for review March 9, 2026 07:00
@saknarajapakshe saknarajapakshe requested review from a team and Copilot March 9, 2026 07:00
Copy link
Copy Markdown

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 delivers the complete React/TypeScript frontend implementation for the pay-slip micro-app, which lets employees view their pay slips and gives admins tools to upload, manage, and view pay slips across all users. The app integrates with a SuperApp native bridge for token-based auth and file downloads, and is bundled as a single HTML file using Vite.

Changes:

  • Added the full frontend source (src/) including hooks, views, components, utilities, and types, covering both user and admin workflows.
  • Added infrastructure files: Vite/Tailwind/TypeScript configuration, global styles, and a development mock server.
  • Added project metadata files (README, microapp.json, .env.example, .gitignore, and version-sync script).

Reviewed changes

Copilot reviewed 40 out of 42 changed files in this pull request and generated 25 comments.

Show a summary per file
File Description
vite.config.ts Vite build config with single-file output; sets unrealistically large size warning limits
tsconfig.json Strict TypeScript configuration
tailwind.config.cjs Tailwind config scanning src/** for classes
styles.css Global base styles + custom slide-up animation
package.json Dependency declarations and npm scripts
index.html App shell with #root mount point
src/index.tsx React root mount
src/App.tsx Main orchestration: auth, view routing, modal management, upload/delete logic
src/types/index.ts Core domain types (User, PaySlip, etc.)
src/types/global.d.ts Window interface augmentation for native bridge
src/constants.ts Centralized config constants (some unused in hooks)
src/api/client.ts Fetch wrapper with retry logic; uploadFile bypasses retry
src/bridge.ts Native bridge wrapper with localStorage fallback
src/hooks/useAuth.ts Token-based auth; loading state can hang if bridge never resolves
src/hooks/useBridge.ts Native bridge token retrieval; ignores BRIDGE_CONFIG constants
src/hooks/usePaySlips.ts Fetches all payslips, filters client-side even when server endpoint exists
src/hooks/useUsers.ts Admin-only user list fetcher
src/views/PaySlipList.tsx User-facing list with filter/sort logic (duplicated across views)
src/views/AllUsersView.tsx Admin user directory
src/views/AdminUserDetailView.tsx Per-user admin view with upload/delete; filter logic duplicated
src/views/AdminPaySlipList.tsx Admin all-payslips view; groups by email (should be userId)
src/components/UI.tsx Button, Card, Input, Select, Modal primitives; uses tailwindcss-animate classes without the plugin
src/components/Filters.tsx Month/year filter toggle; duplicates mergeClassName locally
src/components/PaySlipCard.tsx Payslip card; download button commented out
src/components/PDFViewer.tsx iframe-based PDF viewer; download commented out
src/components/AppPickerModal.tsx View-in-app vs. open-external picker modal
src/components/UploadModal.tsx Admin upload form with duplicate detection
src/components/UserCard.tsx Admin user list card
src/components/LoadingState.tsx Skeleton loading placeholders
src/components/EmptyState.tsx Empty list state component
src/components/ErrorState.tsx Error display with retry button
src/utils/formatters.ts Date/name formatters; contains unused getMonthName function
src/utils/downloadUtils.ts Web file download helpers
src/utils/className.ts Class-merge utility (poorly named, causes import aliasing)
mock-server/server.cjs Express dev mock; ID collision bug after deletes; typo in blob URL
scripts/sync-microapp-version.cjs Keeps microapp.json version in sync with package.json
microapp.json App metadata for the SuperApp registry
README.md Setup/feature docs; references download feature that is commented out
.gitignore / .env.example Standard ignores and env template

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

Comment on lines +123 to +131
const [openGroups, setOpenGroups] = useState<Record<string, boolean>>({});

React.useEffect(() => {
const init: Record<string, boolean> = {};
Object.keys(grouped).forEach((email) => {
init[email] = true; // expand by default
});
setOpenGroups(init);
}, [grouped]);
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

The AdminPaySlipList component initializes openGroups state before the grouped memoized value is computed (line 123), and then uses a useEffect to expand all groups whenever grouped changes (lines 125–131). This approach causes an extra render cycle on every filter change — the groups first render collapsed, then immediately re-render expanded. Since the intent is to always start groups as expanded, it would be cleaner to initialize openGroups directly from grouped computed inline, or to derive the expanded state without storing it in state at all (e.g., tracking collapsed groups instead of expanded ones, defaulting to expanded).

Copilot uses AI. Check for mistakes.
Comment on lines +31 to +49
const currentMonth = new Date().getMonth() + 1;

const [userId, setUserId] = useState(preselectedUserId || "");
const [month, setMonth] = useState<number>(0);
const [year, setYear] = useState<number>(currentYear);
const [file, setFile] = useState<File | null>(null);
const [uploading, setUploading] = useState(false);
const [error, setError] = useState<string | null>(null);

React.useEffect(() => {
if (isOpen) {
setUserId(preselectedUserId || "");
setMonth(0);
setYear(currentYear);
setFile(null);
setError(null);
setUploading(false);
}
}, [isOpen, preselectedUserId, currentMonth, currentYear]);
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

The UploadModal component's useEffect that resets form state on open (lines 40–49) lists currentMonth as a dependency, but currentMonth is computed as new Date().getMonth() + 1 on every render (line 31) and is never used within the effect body. This means a stale-closure linter rule would flag this dependency, but more importantly it indicates the effect was partially refactored (the month was previously reset to currentMonth but that was removed). The currentMonth variable can be removed entirely since it is not used anywhere else in the component.

Copilot uses AI. Check for mistakes.
Comment on lines +14 to +67
(import.meta as any).env?.VITE_DEV_TOKEN || "dev-token-123";
if (!inHost) {
console.warn("Bridge not found. Using dev token.");
setToken(defaultDevToken);
setIsReady(true);
return;
}

// Retry logic for token fetching
const maxRetries = 3;
let retries = 0;

while (retries < maxRetries) {
try {
const fetchedToken = await (
window as any
).nativebridge.requestToken();
if (fetchedToken && fetchedToken.trim() !== "") {
setToken(fetchedToken);
setIsReady(true);
return;
}

// Token is null/empty, retry
console.warn(
`Token not available, retrying... (${retries + 1}/${maxRetries})`,
);
await new Promise((resolve) => setTimeout(resolve, 500));
retries++;
} catch (e) {
console.error("requestToken failed", e);
retries++;

if (retries < maxRetries) {
await new Promise((resolve) => setTimeout(resolve, 500));
}
}
}

// If we get here, we couldn't get a valid token after retries
console.error(
"Failed to obtain token after retries, falling back to dev token",
);
setToken(defaultDevToken);
setIsReady(true);
};
run();
}, [inHost]);

const requestToken = useCallback(async () => {
if (inHost) {
return (window as any).nativebridge.requestToken();
}
return "dev-token-123";
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

The useBridge hook defines retry configuration and default token values inline as magic literals (e.g., 3 for max retries, 500 for retry delay, and the hardcoded string "dev-token-123") instead of using the BRIDGE_CONFIG constants that are defined in src/constants.ts precisely for this purpose. This means the constants and the actual behavior can silently diverge when one is updated without the other.

Additionally, the requestToken fallback (line 67) always returns the hardcoded literal "dev-token-123" even when VITE_DEV_TOKEN is set in the environment, unlike the useEffect path which correctly reads the env variable. This means callers of requestToken() will get a different dev token than the one configured through the environment variable.

Copilot uses AI. Check for mistakes.
Comment on lines +19 to +35
export const getMonthName = (month: number): string => {
const months = [
"January",
"February",
"March",
"April",
"May",
"June",
"July",
"August",
"September",
"October",
"November",
"December",
];
return months[month - 1] || "";
};
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

The getMonthName function in formatters.ts duplicates month name data that already exists in MONTH_NAMES from constants.ts. This function is also unused (no imports of getMonthName exist in the codebase). It should either be removed or re-implemented to reference MONTH_NAMES from constants to avoid duplication.

Copilot uses AI. Check for mistakes.
Comment on lines +21 to +31
const response = await api.getPayslips(token);
const allPayslips = response.data || [];

// If fetching a specific user's payslips (admin detail view),
// filter the results client-side
if (userId && isAdmin) {
setPayslips(allPayslips.filter((slip) => slip.userId === userId));
} else {
setPayslips(allPayslips);
}

Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

The usePaySlips hook always fetches all payslips from /api/pay-slips and then filters them client-side when a userId is provided (line 26–27). The mock server already exposes a dedicated /api/users/:id/pay-slips endpoint that returns only the relevant payslips for a user. Fetching all payslips and filtering on the client is inefficient, especially as the dataset grows — it wastes bandwidth and sends potentially sensitive payslip data for all users across the wire. The hook should use the per-user endpoint when userId is set.

Suggested change
const response = await api.getPayslips(token);
const allPayslips = response.data || [];
// If fetching a specific user's payslips (admin detail view),
// filter the results client-side
if (userId && isAdmin) {
setPayslips(allPayslips.filter((slip) => slip.userId === userId));
} else {
setPayslips(allPayslips);
}
let response;
if (userId && isAdmin) {
// For admin viewing a specific user's payslips, use the per-user endpoint
response = await api.get(`/users/${userId}/pay-slips`, {
headers: { Authorization: `Bearer ${token}` },
});
} else {
// For other cases, fetch all payslips as before
response = await api.getPayslips(token);
}
const payslipData = response?.data || [];
setPayslips(payslipData);

Copilot uses AI. Check for mistakes.
Comment on lines +109 to +110
if (!isOpen) return null;

Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

The UploadModal component has a duplicate guard: if (!isOpen) return null at line 109 and the condition is also checked inside the useEffect at line 41. The early return at line 109 means the component correctly short-circuits, but the outer Modal component at line 112 also guards on isOpen (it returns null if !isOpen). Having the guard in both places is redundant — the if (!isOpen) return null at line 109 in UploadModal is effectively unreachable because the Modal wrapping it handles the non-open case already. If the intent is to prevent state from being initialized, having the modal always render and just hiding it via Modal's internal guard is the right approach, in which case the guard at line 109 should be removed.

Suggested change
if (!isOpen) return null;

Copilot uses AI. Check for mistakes.
Comment on lines +131 to +146
return (
<div className="fixed inset-0 z-50 flex items-center justify-center p-4 bg-slate-900/50 backdrop-blur-sm animate-in fade-in duration-200">
<div className="bg-white rounded-2xl shadow-xl w-full max-w-sm overflow-hidden animate-in zoom-in-95 duration-200">
<div className="flex justify-between items-center p-4 border-b border-slate-100">
<h3 className="font-semibold text-slate-800">{title}</h3>
<button
onClick={onClose}
className="text-slate-400 hover:text-slate-600"
>
<X size={20} />
</button>
</div>
<div className="p-4">{children}</div>
</div>
</div>
);
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

The Modal component renders a backdrop that can be clicked (as the entire fixed overlay is rendered), but there is no onClick handler on the backdrop to dismiss the modal when the user clicks outside the modal panel. The close button is the only way to dismiss the modal. This is an inconsistency with typical modal UX expectations and may frustrate users who try clicking outside the modal to close it. The AppPickerModal, UploadModal, and the delete confirmation modal are all affected. Consider adding a click-outside handler on the backdrop div.

Copilot uses AI. Check for mistakes.
Comment on lines +42 to +43
assetsInlineLimit: 100000000,
chunkSizeWarningLimit: 100000000,
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

The vite.config.ts sets assetsInlineLimit: 100000000 and chunkSizeWarningLimit: 100000000 (100MB) — these are extremely large values chosen to suppress all size warnings rather than genuinely set appropriate limits. The intent is to produce a single-file output (via vite-plugin-singlefile), but suppressing warnings by inflating the limit to 100MB hides legitimate warnings if the bundle grows unexpectedly large. Consider removing these overrides or setting a more realistic threshold (e.g., 5MB) while still using the singlefile plugin.

Copilot uses AI. Check for mistakes.
Comment on lines +69 to +84
const handleOpenExternal = async () => {
if (!selectedPayslip) return;

try {
// Use the native bridge to open with external app
await requestDownloadFile({
url: selectedPayslip.fileUrl,
filename: `payslip-${selectedPayslip.month}-${selectedPayslip.year}.pdf`,
});
} catch (error) {
console.error("Failed to open with external app:", error);
} finally {
setShowPicker(false);
setSelectedPayslip(null);
}
};
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

The handleOpenExternal function in PaySlipList.tsx (and duplicated in AdminPaySlipList.tsx and AdminUserDetailView.tsx) calls setSelectedPayslip(null) in the finally block (line 82). However, if requestDownloadFile is called and the download succeeds, setShowPicker(false) is called via finally — but the viewer may remain open for the same payslip since selectedPayslip is cleared. This is correct behavior here. However, there's a subtle issue: if the native download is triggered but fails (caught by the catch), selectedPayslip is still cleared and the user has no way to retry without re-selecting the payslip. Consider keeping selectedPayslip in state on failure so the user can try again.

Copilot uses AI. Check for mistakes.
Comment on lines +61 to +67
const {
payslips: userPayslips,
loading: userPayslipsLoading,
error: userPayslipsError,
refresh: refreshUserPayslips,
} = usePaySlips({
token,
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

The App.tsx always calls usePaySlips twice unconditionally — once for the global list and once for the per-user detail view (lines 32–40 and 61–70). The per-user hook is initialized with userId: selectedUser?.id || null even before any user is selected, and when userId is null, the hook fetches all payslips again (because the userId && isAdmin guard in usePaySlips is false when userId is null). This means the app makes two identical fetch requests to /api/pay-slips on initial load. The per-user hook instance should only be initialized when selectedUser is non-null, or the hook should skip fetching when userId is null and isAdmin is false.

Suggested change
const {
payslips: userPayslips,
loading: userPayslipsLoading,
error: userPayslipsError,
refresh: refreshUserPayslips,
} = usePaySlips({
token,
const userPayslipsToken = selectedUser ? token : null;
const {
payslips: userPayslips,
loading: userPayslipsLoading,
error: userPayslipsError,
refresh: refreshUserPayslips,
} = usePaySlips({
token: userPayslipsToken,

Copilot uses AI. Check for mistakes.
@saknarajapakshe saknarajapakshe force-pushed the feat/payslip-frontend branch from b202371 to 57c7202 Compare March 9, 2026 10:19
Copy link
Copy Markdown

@ginaxu1 ginaxu1 left a comment

Choose a reason for hiding this comment

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

Good to check for duplicate logic - follow "Don't repeat yourself" (DRY) principle. For example PaySlipList.tsx and AdminUserDetailView.tsx have nearly same useMemo blocks for filtering (by month/year) and sorting (descending by date) payslips. Better to extract into shared utility function or a custom hook like useFilteredPaySlips

Suggestions for future, to also make your life easier + code review more efficient:

  • If you're using AI powered IDE, ask agent to do a code review to ensure your code follows DRY, Proper error handling, Type safety and no Antipatterns - address those issues before you create the PR.
  • It's good practice to split these big features into smaller PRs next time (typically try to keep PR code changes around 400 lines). One way you could have split this frontend feature: 1. create the infra and type definitions, 2. create networking layer + base helper functions src/api/client.s src/utils/*, 3. wire up the superapp integration 4. create shared ui components 5. add the src/hooks and other src/components

- Add package.json for project dependencies and scripts
- Create styles.css with minimal styling for the application
- Set up TypeScript configuration in tsconfig.json
- Configure Vite for building and serving the application

feat: update project configuration and improve error handling

- Enhance .gitignore to include additional files and directories
- Improve error handling in localStorage access within bridge.ts
- Remove unused API_BASE constant from constants.ts
- Add predev and prebuild scripts to package.json for version syncing
- Update TypeScript and React type definitions in package.json
- Add postcss.config.js for Tailwind CSS configuration
- Implement sync-microapp-version scripts for version management
- Refactor styles.css to utilize Tailwind CSS utilities
- Add Tailwind CSS configuration file

feat: set up initial frontend structure with React components and configuration
…d reusable UI components

feat: addressed reviewer comments

fix :address minor issues in admin side

feat: update PaySlipCard to temporarily disable download functionality
…nality

- Added "My Slips" view for admin users to manage their own pay slips.
- Implemented floating upload button for admin users in the "All Slips" view.
- Updated API client to support pagination for fetching pay slips.
- Refactored pay slip filtering logic to improve performance and clarity.
- Enhanced user detail view to allow for better management of individual user pay slips.
- Introduced year and month filters in pay slip lists for better navigation.
- Updated UI components for improved user experience and consistency.
- Adjusted file upload handling to use new API response structure.

feat: enhance PaySlipList with refresh button and improved layout for filters
<div className="min-w-0">
<h1 className="text-2xl leading-tight font-semibold text-slate-900 truncate">
{selectedUser.email
.split("@")[0]
Copy link
Copy Markdown

@ginaxu1 ginaxu1 Mar 11, 2026

Choose a reason for hiding this comment

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

App.tsx assumes the selectedUser always has an email and that the email follows a specific name.surname@domain format. If an email is missing or formatted differently, the app could crash or display broken text. better move this logic (.split("@")[0].replace(/\./g, " ") ) into a dedicated utility function in src/utils/formatters.ts to keep the component clean, and there you can add safety checks such as returning a fallback string if the email is undefined or empty, to protect the UI from breaking

setDeleteError(null);
};

const handleConfirmDelete = async () => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

handleConfirmDelete function catches errors and sets a deleteError state. While this shows an error in the modal, doesn't provide a clear way for the user to "reset" or understand if the error is transient. Also, if modal is closed while deletingPayslip is true, the state might not reset correctly

suggest: add a "Retry" button within the delete confirmation modal if an error occurs. Then make sure that closeDeleteConfirm properly clears all delete-related states (deleteConfirmId, deleteError, deletingPayslip) to prevent stale error messages from appearing when the user tries to delete a different item later

const [loading, setLoading] = useState(false);
const [error, setError] = useState<string | null>(null);

const fetchPayslips = useCallback(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

i think there's potential memory leak : if fetchPayslips is called and the user quickly switches views (like from "My Slips" to "Employees"), the network request is still running in the background. When the request finally finishes, it tries to update the state (setPayslips) of a component that is no longer on the screen. This can cause "Warning: Can't perform a React state update on an unmounted component" errors and wastes system memory

suggest: add an AbortController to the fetchPayslips function to cancel any ongoing fetch requests if the component unmounts or if a new request is started before the previous one completes.

@sthanikan2000 sthanikan2000 marked this pull request as draft March 31, 2026 10:25
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