feat: add feature toggle for notification banner #93
Conversation
Signed-off-by: joshydavid <cxang.2022@scis.smu.edu.sg>
Signed-off-by: joshydavid <cxang.2022@scis.smu.edu.sg>
Signed-off-by: joshydavid <cxang.2022@scis.smu.edu.sg>
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds a feature toggle for controlling the visibility of a notification banner in the header component. The toggle is implemented using an environment variable that allows the banner to be conditionally displayed.
- Added environment variable-based feature toggle for notification banner
- Wrapped existing banner component with conditional rendering logic
- Updated environment sample file with the new configuration option
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| frontend/app/components/Header/index.tsx | Added conditional rendering logic for banner using environment variable |
| frontend/.env.sample | Added new environment variable for notification banner toggle |
| export default function Header() { | ||
| const pathname = usePathname(); | ||
| const { authData } = useAuthStatus(); | ||
| const showBanner = process.env.NEXT_PUBLIC_NOTIFICATION_BANNER === "true"; |
There was a problem hiding this comment.
Consider using a more robust configuration approach. Direct environment variable access makes testing difficult and creates tight coupling. Consider creating a configuration utility function or using a configuration context that can be easily mocked in tests.
| title="⚠️ Notice: AWS resources have been spun down. The backend is | ||
| currently not running." |
There was a problem hiding this comment.
The hardcoded banner message creates tight coupling between the feature toggle and specific content. Consider making the banner message configurable through environment variables or a separate configuration, allowing the same toggle mechanism to be reused for different notifications.
| title="⚠️ Notice: AWS resources have been spun down. The backend is | |
| currently not running." | |
| title={ | |
| process.env.NEXT_PUBLIC_BANNER_MESSAGE || | |
| "⚠️ Notice: AWS resources have been spun down. The backend is currently not running." | |
| } |
Signed-off-by: joshydavid <cxang.2022@scis.smu.edu.sg>
|
1 participant
Description
What type of PR is this?
Desktop and mobile screenshots/recordings