Skip to content

Conversation

nxglabs
Copy link
Collaborator

@nxglabs nxglabs commented Sep 8, 2025

Merge pull request

Copy link

vercel bot commented Sep 8, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
open-sign Ready Ready Preview Comment Sep 8, 2025 4:59am
@Copilot Copilot AI review requested due to automatic review settings September 8, 2025 04:57
@prafull-opensignlabs prafull-opensignlabs changed the title Merge pull request #1250 from nxglabs/sync-to-public_repo-17512737859 v2.27.2 Sep 8, 2025
Copy link

@Copilot 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 implements a comprehensive contact management system for OpenSign along with various improvements to the UI and codebase structure. The primary focus is adding support for importing, managing, and organizing contacts within the application.

Key changes:

  • Contact Management System: Adds complete CRUD operations for contacts with import/export capabilities, additional fields (Company, JobTitle), and enhanced contact forms
  • Redux State Refactoring: Replaces showHeader reducer with sidebarReducer and adds new widget state management for prefill functionality
  • UI/UX Enhancements: Improves modals, user deletion workflows, session management, and PDF verification features

Reviewed Changes

Copilot reviewed 109 out of 176 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
DocumentsReport.jsx New comprehensive document reporting component with advanced filtering and actions
ImportContact.jsx Contact import functionality supporting CSV/Excel with validation
EditContactForm.jsx Enhanced contact editing with additional fields (Company, JobTitle)
Contactbook.jsx Main contact management interface with search, pagination, and CRUD operations
store.js Updated Redux store configuration replacing showHeader with sidebarReducer
widgetSlice.js Enhanced widget state management with prefill image handling
sidebarReducer.js New sidebar state management replacing showHeader functionality
Multiple primitives Various UI component improvements including modals, validation, and user management

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

if (file) {
const fileName = file.name;
const fileNameExt = fileName
.substr(fileName.lastIndexOf(".") + 1)
Copy link

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

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

Use substring() or slice() instead of the deprecated substr() method. Replace with fileName.substring(fileName.lastIndexOf('.') + 1).toLowerCase()

Suggested change
.substr(fileName.lastIndexOf(".") + 1)
.substring(fileName.lastIndexOf(".") + 1)

Copilot uses AI. Check for mistakes.

const { t } = useTranslation();
const [isCheck, setIsCheck] = useState(false);
const [isCheck, setIsCheck] = useState(true);
Copy link

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

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

The initial state should be false to match the original behavior. Setting it to true by default will change the existing functionality.

Suggested change
const [isCheck, setIsCheck] = useState(true);
const [isCheck, setIsCheck] = useState(false);

Copilot uses AI. Check for mistakes.

id="phone"
value={phone}
onChange={(e) => setPhone(e.target.value)}
// disabled={addYourself}
Copy link

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

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

Remove commented-out code or provide a clear explanation for why it's preserved. Commented code should be cleaned up before merging.

Copilot uses AI. Check for mistakes.

Comment on lines +253 to +254
if (!Number.isFinite(num)) {
// Checks for NaN, Infinity, -Infinity
Copy link

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider using Number.isSafeInteger(num) for stricter validation of PDF byte range values, as PDF specifications typically expect safe integer values.

Suggested change
if (!Number.isFinite(num)) {
// Checks for NaN, Infinity, -Infinity
if (!Number.isSafeInteger(num)) {
// Checks for NaN, Infinity, -Infinity, and non-integer or unsafe integer values

Copilot uses AI. Check for mistakes.

Comment on lines +24 to +26
let UserProfile =
localStorage.getItem("UserInformation") &&
JSON.parse(localStorage.getItem("UserInformation"));
Copy link

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

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

Avoid calling localStorage.getItem('UserInformation') twice. Store the result in a variable first: const userInfo = localStorage.getItem('UserInformation'); let UserProfile = userInfo && JSON.parse(userInfo);

Suggested change
let UserProfile =
localStorage.getItem("UserInformation") &&
JSON.parse(localStorage.getItem("UserInformation"));
const userInfo = localStorage.getItem("UserInformation");
let UserProfile = userInfo && JSON.parse(userInfo);

Copilot uses AI. Check for mistakes.

await onConfirm();
} catch (err) {
console.log("err ", err);
setError(err?.message || t("something-went-wron-mssg"));
Copy link

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

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

Fix typo in translation key: 'something-went-wron-mssg' should be 'something-went-wrong-mssg'

Suggested change
setError(err?.message || t("something-went-wron-mssg"));
setError(err?.message || t("something-went-wrong-mssg"));

Copilot uses AI. Check for mistakes.

@prafull-opensignlabs prafull-opensignlabs merged commit 1c2f453 into staging Sep 8, 2025
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants