-
Notifications
You must be signed in to change notification settings - Fork 21
[TEST DRAFT] #1262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[TEST DRAFT] #1262
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| name: AI PR Reviewer Updated | ||
|
|
||
| on: | ||
| pull_request: | ||
| types: | ||
| - opened | ||
| - synchronize | ||
| permissions: | ||
| pull-requests: write | ||
| jobs: | ||
| tc-ai-pr-review: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout Repo | ||
| uses: actions/checkout@v3 | ||
|
|
||
| - name: TC AI PR Reviewer | ||
| uses: topcoder-platform/tc-ai-pr-reviewer@prompt-update | ||
| with: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # The GITHUB_TOKEN is there by default so you just need to keep it like it is and not necessarily need to add it as secret as it will throw an error. [More Details](https://docs.github.com/en/actions/security-guides/automatic-token-authentication#about-the-github_token-secret) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [💡 |
||
| LAB45_API_KEY: ${{ secrets.LAB45_API_KEY }} | ||
| exclude: '**/*.json, **/*.md, **/*.jpg, **/*.png, **/*.jpeg, **/*.bmp, **/*.webp' # Optional: exclude patterns separated by commas | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The exclude pattern is optional, but it might be beneficial to ensure that it aligns with the project's requirements. Double-check that the patterns listed here are indeed the ones you want to exclude from the AI PR review process. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ import { TableLoading } from '~/apps/admin/src/lib' | |
| import { handleError } from '~/apps/admin/src/lib/utils' | ||
| import { EnvironmentConfig } from '~/config' | ||
| import { BaseModal, Button, InputCheckbox, InputText } from '~/libs/ui' | ||
| import { NotificationContextType, useNotification } from '~/libs/shared' | ||
|
|
||
| import { | ||
| useFetchScreeningReview, | ||
|
|
@@ -226,6 +227,7 @@ const computePhaseCompletionFromScreenings = ( | |
|
|
||
| // eslint-disable-next-line complexity | ||
| export const ChallengeDetailsPage: FC<Props> = (props: Props) => { | ||
| const { showBannerNotification, removeNotification }: NotificationContextType = useNotification() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [💡 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The destructuring of |
||
| const [searchParams, setSearchParams] = useSearchParams() | ||
| const location = useLocation() | ||
| const navigate = useNavigate() | ||
|
|
@@ -1323,6 +1325,16 @@ export const ChallengeDetailsPage: FC<Props> = (props: Props) => { | |
| : undefined | ||
| const shouldShowChallengeMetaRow = Boolean(statusLabel) || trackTypePills.length > 0 | ||
|
|
||
| useEffect(() => { | ||
| const notification = showBannerNotification({ | ||
| id: 'ai-review-scores-warning', | ||
| message: `AI Review Scores are advisory only to provide immediate, | ||
| educational, and actionable feedback to members. | ||
| AI Review Scores are not influence winner selection.`, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [❗❗ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typographical error in the message: 'AI Review Scores are not influence winner selection.' should be 'AI Review Scores do not influence winner selection.' |
||
| }) | ||
| return () => notification && removeNotification(notification.id) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider checking if |
||
| }, [showBannerNotification]) | ||
|
|
||
| return ( | ||
| <PageWrapper | ||
| pageTitle={challengeInfo?.name ?? ''} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,3 @@ | ||
| export * from './profile-context-data.model' | ||
| export { default as profileContext, defaultProfileContextData } from './profile.context' | ||
| export { default as profileContext, defaultProfileContextData, useProfileContext } from './profile.context' | ||
| export * from './profile.context-provider' |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import { Context, createContext } from 'react' | ||
| import { Context, createContext, useContext } from 'react' | ||
|
|
||
| import { ProfileContextData } from './profile-context-data.model' | ||
|
|
||
|
|
@@ -12,4 +12,6 @@ export const defaultProfileContextData: ProfileContextData = { | |
|
|
||
| const profileContext: Context<ProfileContextData> = createContext(defaultProfileContextData) | ||
|
|
||
| export const useProfileContext = (): ProfileContextData => useContext(profileContext) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ |
||
|
|
||
| export default profileContext | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| import { FC } from 'react' | ||
|
|
||
| import { Notification } from '~/libs/ui' | ||
|
|
||
| import { NotificationContextType, useNotification } from './Notifications.context' | ||
| import styles from './NotificationsContainer.module.scss' | ||
|
|
||
| const NotificationsContainer: FC = () => { | ||
| const { notifications, removeNotification }: NotificationContextType = useNotification() | ||
|
|
||
| return ( | ||
| <div className={styles.wrap}> | ||
| {notifications.map(n => ( | ||
| <Notification key={n.id} notification={n} onClose={removeNotification} /> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [💡 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider renaming the variable |
||
| ))} | ||
| </div> | ||
| ) | ||
| } | ||
|
|
||
| export default NotificationsContainer | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| import React, { createContext, ReactNode, useCallback, useContext, useMemo, useState } from 'react' | ||
|
|
||
| import { useProfileContext } from '~/libs/core' | ||
|
|
||
| import { dismiss, wasDismissed } from './localstorage.utils' | ||
|
|
||
| export type NotificationType = 'success' | 'error' | 'info' | 'warning' | 'banner'; | ||
|
|
||
| export interface Notification { | ||
| id: string; | ||
| type: NotificationType; | ||
| icon?: ReactNode | ||
| message: string; | ||
| duration?: number; // in ms | ||
| } | ||
|
|
||
| type NotifyPayload = string | (Partial<Notification> & { message: string }) | ||
|
|
||
| export interface NotificationContextType { | ||
| notifications: Notification[]; | ||
| notify: (message: NotifyPayload, type?: NotificationType, duration?: number) => Notification | void; | ||
| showBannerNotification: (message: NotifyPayload) => Notification | void; | ||
| removeNotification: (id: string) => void; | ||
| } | ||
|
|
||
| const NotificationContext = createContext<NotificationContextType | undefined>(undefined) | ||
|
|
||
| export const useNotification = (): NotificationContextType => { | ||
| const context = useContext(NotificationContext) | ||
| if (!context) throw new Error('useNotification must be used within a NotificationProvider') | ||
| return context | ||
| } | ||
|
|
||
| export const NotificationProvider: React.FC<{ | ||
| children: ReactNode, | ||
| }> = props => { | ||
| const profileCtx = useProfileContext() | ||
| const uuid = profileCtx.profile?.userId ?? 'annon' | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ |
||
| const [notifications, setNotifications] = useState<Notification[]>([]) | ||
|
|
||
| const removeNotification = useCallback((id: string, persist?: boolean) => { | ||
| setNotifications(prev => prev.filter(n => n.id !== id)) | ||
| if (persist) { | ||
| dismiss(id) | ||
| } | ||
| }, []) | ||
|
|
||
| const notify = useCallback( | ||
| (message: NotifyPayload, type: NotificationType = 'info', duration = 3000) => { | ||
| const id = `${uuid}[${typeof message === 'string' ? message : message.id}]` | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| const newNotification: Notification | ||
| = typeof message === 'string' | ||
| ? { duration, id, message, type } | ||
| : { duration, type, ...message, id } | ||
|
|
||
| if (wasDismissed(id)) { | ||
| return undefined | ||
| } | ||
|
|
||
| setNotifications(prev => [...prev, newNotification]) | ||
|
|
||
| if (duration > 0) { | ||
| setTimeout(() => removeNotification(id), duration) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| } | ||
|
|
||
| return newNotification | ||
| }, | ||
| [uuid], | ||
| ) | ||
|
|
||
| const showBannerNotification = useCallback(( | ||
| message: NotifyPayload, | ||
| ) => notify(message, 'banner', 0), [notify]) | ||
|
|
||
| const ctxValue = useMemo(() => ({ | ||
| notifications, | ||
| notify, | ||
| removeNotification, | ||
| showBannerNotification, | ||
| }), [ | ||
| notifications, | ||
| notify, | ||
| removeNotification, | ||
| showBannerNotification, | ||
| ]) | ||
|
|
||
| return ( | ||
| <NotificationContext.Provider value={ctxValue}> | ||
| {props.children} | ||
| </NotificationContext.Provider> | ||
| ) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| @import "@libs/ui/styles/includes"; | ||
|
|
||
| .wrap { | ||
| position: relative; | ||
| width: 100%; | ||
| z-index: 1000; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| export { default as NotificationsContainer } from './Notifications.container' | ||
| export * from './Notifications.context' |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| const lsKeyPrefix = 'notificationDismissed' | ||
|
|
||
| export const wasDismissed = (id: string): boolean => ( | ||
| (localStorage.getItem(`${lsKeyPrefix}[${id}]`)) !== null | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [❗❗ |
||
| ) | ||
|
|
||
| export const dismiss = (id: string): void => { | ||
| localStorage.setItem(`${lsKeyPrefix}[${id}]`, JSON.stringify(true)) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [❗❗ |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| import { FC, ReactNode, useCallback } from 'react' | ||
|
|
||
| import { NotificationBanner } from './banner' | ||
|
|
||
| interface NotificationProps { | ||
| notification: { | ||
| icon?: ReactNode; | ||
| id: string; | ||
| message: string; | ||
| type: string; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using a union type for |
||
| } | ||
| onClose: (id: string, save?: boolean) => void | ||
| } | ||
|
|
||
| const Notification: FC<NotificationProps> = props => { | ||
| const handleClose = useCallback((save?: boolean) => { | ||
| props.onClose(props.notification.id, save) | ||
| }, [props.onClose, props.notification.id]) | ||
|
|
||
| if (props.notification.type === 'banner') { | ||
| return ( | ||
| <NotificationBanner | ||
| icon={props.notification.icon} | ||
| content={props.notification.message} | ||
| onClose={handleClose} | ||
| /> | ||
| ) | ||
| } | ||
|
|
||
| return <></> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Returning an empty fragment |
||
| } | ||
|
|
||
| export default Notification | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| @import '../../../styles/includes'; | ||
|
|
||
| .wrap { | ||
| background: #60267D; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using a variable for the background color instead of a hardcoded hex value to maintain consistency and ease of updates across the project. |
||
| color: $tc-white; | ||
|
|
||
| font-family: "Nunito Sans", sans-serif; | ||
| font-size: 14px; | ||
| line-height: 20px; | ||
|
|
||
| .inner { | ||
| max-width: $xxl-min; | ||
| padding: $sp-2 0; | ||
| @include pagePaddings; | ||
| margin: 0 auto; | ||
| width: 100%; | ||
| display: flex; | ||
| justify-content: space-between; | ||
| align-items: center; | ||
| } | ||
| } | ||
|
|
||
| .close { | ||
| cursor: pointer; | ||
| color: $tc-white; | ||
| flex: 0 0; | ||
| margin-left: auto; | ||
| border-radius: 50%; | ||
| border: 2px solid white; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using a variable for the border color instead of hardcoding 'white'. This will help maintain consistency and make future updates easier. |
||
| @include ltemd { | ||
| margin-left: $sp-3; | ||
| } | ||
| } | ||
|
|
||
| .icon { | ||
| flex: 0 0; | ||
| margin-right: $sp-2; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import { Meta, StoryObj } from '@storybook/react' | ||
|
|
||
| import NotificationBanner from './NotificationBanner' | ||
|
|
||
| const meta: Meta<typeof NotificationBanner> = { | ||
| argTypes: { | ||
| content: { | ||
| description: 'Content displayed inside the notification banner', | ||
| }, | ||
| persistent: { | ||
| defaultValue: false, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ |
||
| description: 'Set to true to hide the close icon button', | ||
| }, | ||
| }, | ||
| component: NotificationBanner, | ||
| excludeStories: /.*Decorator$/, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [💡 |
||
| tags: ['autodocs'], | ||
| title: 'Components/NotificationBanner', | ||
| } | ||
|
|
||
| export default meta | ||
|
|
||
| type Story = StoryObj<typeof NotificationBanner>; | ||
|
|
||
| export const Primary: Story = { | ||
| args: { | ||
| content: 'Help tooltip', | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| }, | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment about the
GITHUB_TOKENis informative but could be misleading if it suggests that no action is needed. Consider rephrasing to clarify that the token is automatically provided by GitHub Actions and does not need to be manually added as a secret.