Skip to content

Conversation

@vivekyadav-3
Copy link

This PR addresses issues #154 and #157 by:

  1. Adding auto-hide logic to the legacy Alert component in Widgets/Alert.
  2. Implementing a modern useNotification hook using Sonner for a consistent API.
  3. Refactoring the Home page (HomeClient.jsx) to use the new hook, improving code readability and reducing boilerplate.
  4. Correctly integrating the <Toaster /> component in the root layout.
    Fixes Hide Error Snackbar after couple of seconds #154
    Fixes Hooks for handling notifications and error snack bars #157

Copilot AI review requested due to automatic review settings December 26, 2025 05:29
Copy link

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 modernizes the notification system by implementing a reusable useNotification hook built on Sonner and adding auto-hide functionality to existing alerts. It addresses issues #154 and #157 by providing a consistent API for notifications and improving user experience with automatic dismissal.

Key Changes:

  • Implemented useNotification hook with customizable success, error, info, and warning notifications
  • Added 5-second auto-hide timer to the legacy Alert component
  • Refactored HomeClient to use the new hook, reducing boilerplate code

Reviewed changes

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

Show a summary per file
File Description
src/styles/global.css Added CSS variables for notification colors and improved code formatting
src/hooks/use-notification.js New hook providing a unified API for notifications using Sonner toast library
src/components/Widgets/Alert/index.jsx Added auto-hide functionality with 5-second timer using useEffect
src/app/layout.jsx Integrated Toaster component in root layout to enable Sonner notifications
src/app/HomeClient/HomeClient.jsx Replaced legacy alert state management with new useNotification hook

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

Comment on lines +21 to +65
export const useNotification = () => {
const notify = (message, options = {}) => {
return toast(message, options);
};

const success = (message, options = {}) => {
return toast.success(message, {
...options,
style: {
background: 'var(--success-bg, #e6ffed)',
borderColor: 'var(--success-border, #b7eb8f)',
color: 'var(--success-text, #14532d)',
},
});
};

const error = (message, options = {}) => {
return toast.error(message, {
duration: 5000, // Addresses Issue #154 in the new hook
...options,
style: {
background: 'var(--error-bg, #fff1f0)',
borderColor: 'var(--error-border, #ffa39e)',
color: 'var(--error-text, #721c24)',
},
});
};

const info = (message, options = {}) => {
return toast.info(message, options);
};

const warning = (message, options = {}) => {
return toast.warning(message, options);
};

return {
notify,
success,
error,
info,
warning,
dismiss: toast.dismiss,
};
};
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

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

The useNotification hook creates new function instances on every render, which could impact performance when used in components that render frequently. Consider using useMemo or useCallback to memoize the returned object and functions, or restructure to directly export the wrapper functions instead of wrapping them in a hook.

Copilot uses AI. Check for mistakes.
Comment on lines +27 to +44
return toast.success(message, {
...options,
style: {
background: 'var(--success-bg, #e6ffed)',
borderColor: 'var(--success-border, #b7eb8f)',
color: 'var(--success-text, #14532d)',
},
});
};

const error = (message, options = {}) => {
return toast.error(message, {
duration: 5000, // Addresses Issue #154 in the new hook
...options,
style: {
background: 'var(--error-bg, #fff1f0)',
borderColor: 'var(--error-border, #ffa39e)',
color: 'var(--error-text, #721c24)',
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

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

The style object is placed after the spread operator, which means it will override any user-provided styles. This prevents customization of notification appearance. Consider merging user-provided styles with default styles instead of completely overriding them, or place the style object before the spread operator to allow user customization.

Suggested change
return toast.success(message, {
...options,
style: {
background: 'var(--success-bg, #e6ffed)',
borderColor: 'var(--success-border, #b7eb8f)',
color: 'var(--success-text, #14532d)',
},
});
};
const error = (message, options = {}) => {
return toast.error(message, {
duration: 5000, // Addresses Issue #154 in the new hook
...options,
style: {
background: 'var(--error-bg, #fff1f0)',
borderColor: 'var(--error-border, #ffa39e)',
color: 'var(--error-text, #721c24)',
const { style: userStyle, ...restOptions } = options;
return toast.success(message, {
...restOptions,
style: {
background: 'var(--success-bg, #e6ffed)',
borderColor: 'var(--success-border, #b7eb8f)',
color: 'var(--success-text, #14532d)',
...userStyle,
},
});
};
const error = (message, options = {}) => {
const { style: userStyle, ...restOptions } = options;
return toast.error(message, {
duration: 5000, // Addresses Issue #154 in the new hook
...restOptions,
style: {
background: 'var(--error-bg, #fff1f0)',
borderColor: 'var(--error-border, #ffa39e)',
color: 'var(--error-text, #721c24)',
...userStyle,

Copilot uses AI. Check for mistakes.
Comment on lines +27 to +44
return toast.success(message, {
...options,
style: {
background: 'var(--success-bg, #e6ffed)',
borderColor: 'var(--success-border, #b7eb8f)',
color: 'var(--success-text, #14532d)',
},
});
};

const error = (message, options = {}) => {
return toast.error(message, {
duration: 5000, // Addresses Issue #154 in the new hook
...options,
style: {
background: 'var(--error-bg, #fff1f0)',
borderColor: 'var(--error-border, #ffa39e)',
color: 'var(--error-text, #721c24)',
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

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

The style object is placed after the spread operator, which means it will override any user-provided styles. This prevents customization of notification appearance. Consider merging user-provided styles with default styles instead of completely overriding them, or place the style object before the spread operator to allow user customization.

Suggested change
return toast.success(message, {
...options,
style: {
background: 'var(--success-bg, #e6ffed)',
borderColor: 'var(--success-border, #b7eb8f)',
color: 'var(--success-text, #14532d)',
},
});
};
const error = (message, options = {}) => {
return toast.error(message, {
duration: 5000, // Addresses Issue #154 in the new hook
...options,
style: {
background: 'var(--error-bg, #fff1f0)',
borderColor: 'var(--error-border, #ffa39e)',
color: 'var(--error-text, #721c24)',
const { style: userStyle, ...restOptions } = options;
return toast.success(message, {
...restOptions,
style: {
background: 'var(--success-bg, #e6ffed)',
borderColor: 'var(--success-border, #b7eb8f)',
color: 'var(--success-text, #14532d)',
...userStyle,
},
});
};
const error = (message, options = {}) => {
const { style: userStyle, ...restOptions } = options;
return toast.error(message, {
duration: 5000, // Addresses Issue #154 in the new hook
...restOptions,
style: {
background: 'var(--error-bg, #fff1f0)',
borderColor: 'var(--error-border, #ffa39e)',
color: 'var(--error-text, #721c24)',
...userStyle,

Copilot uses AI. Check for mistakes.
}, 5000);

return () => clearTimeout(timer);
}, [setShow]);
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

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

The useEffect dependency array is missing 'message' and 'type' props. While the current implementation works, React best practices require including all values from the component scope that are used inside the effect. This ensures the timer is reset when these values change, which is important for the auto-hide behavior to work correctly if the alert is reused with different messages.

Suggested change
}, [setShow]);
}, [setShow, message, type]);

Copilot uses AI. Check for mistakes.
* This file contains all styling which is common for all theme (which defines basic layout)
*/
@import 'tailwindcss' important;
@import "tailwindcss" important;
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

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

The @import syntax is incorrect. The 'important' keyword should be preceded by a layer name or removed entirely. The correct syntax should be either '@import "tailwindcss";' or '@import "tailwindcss" layer(important);' depending on the intended behavior. The current syntax will likely cause CSS parsing errors.

Suggested change
@import "tailwindcss" important;
@import "tailwindcss" layer(important);

Copilot uses AI. Check for mistakes.
error(message);
router.replace(window.location.pathname);
}
}, [searchParams, router]);
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

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

The useEffect hook is missing 'error' in its dependency array. Since 'error' is obtained from useNotification() and is used within the effect, it should be included in the dependencies to ensure the effect uses the current reference. This could lead to stale closure issues.

Suggested change
}, [searchParams, router]);
}, [searchParams, router, error]);

Copilot uses AI. Check for mistakes.
Comment on lines +39 to +40
duration: 5000, // Addresses Issue #154 in the new hook
...options,
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

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

The duration option is placed before the spread operator, which means user-provided options can override the 5000ms duration. This defeats the purpose of addressing Issue #154 for auto-hiding error notifications. Consider placing duration after the spread operator to ensure it cannot be overridden, or document this behavior if it's intentional.

Suggested change
duration: 5000, // Addresses Issue #154 in the new hook
...options,
...options,
duration: 5000, // Addresses Issue #154 in the new hook

Copilot uses AI. Check for mistakes.
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.

Hooks for handling notifications and error snack bars Hide Error Snackbar after couple of seconds

2 participants