Skip to content

Conversation

@hamo-o
Copy link
Contributor

@hamo-o hamo-o commented Feb 24, 2025

#️⃣ 연관된 이슈>

📝 작업 내용> 이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)

헤더 드롭다운을 구현합니다. 드디어 부끄러운 닉네임을 바꿀 수 있습니다.

image

🙏 여기는 꼭 봐주세요! > 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요

Summary by CodeRabbit

  • New Features

    • Enhanced avatar components now support click actions and display fallback images, ensuring a consistent visual experience.
    • Introduced a dedicated user avatar dropdown for nickname updates and logout functionality.
    • Added global modal integration for streamlined, app-wide dialog interactions.
    • New method for updating user nicknames has been added, along with a custom hook for managing nickname mutations.
  • Refactor

    • Redesigned dropdown and input components for improved flexibility and maintainability.
    • Simplified modal state management for more reliable displays.
  • Style

    • Updated navigation and interactive element styling for a more polished user experience.

@hamo-o hamo-o added the 🖥️ FE Frontend label Feb 24, 2025
@hamo-o hamo-o added this to the 六 6차 스프린트 milestone Feb 24, 2025
@hamo-o hamo-o requested a review from dioo1461 as a code owner February 24, 2025 05:22
@coderabbitai
Copy link

coderabbitai bot commented Feb 24, 2025

Warning

Rate limit exceeded

@hamo-o has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 13 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between a8a2c77 and 3e2b76e.

📒 Files selected for processing (1)
  • frontend/src/hooks/useClickOutside.ts (1 hunks)

Walkthrough

Multiple components and utilities have been updated across the codebase. Avatar components now accept null image sources with fallback images, and Dropdown components are enhanced with a dedicated contents wrapper and optional properties. Input components gain additional styling support, while Modal handling is streamlined via a global hook and simplified interfaces. Discussion form dropdowns have been refactored for improved structure. User API capabilities now include a PATCH method and nickname mutation hook, and global state and utility functions are extended with new features.

Changes

File(s) Change Summary
frontend/src/components/Avatar/AvatarItem.tsx, .../Avatar/index.tsx Updated Avatar components: modified src type to allow null, added optional onClick in Avatar, and implemented fallback image logic.
.../components/Dropdown/* Redesigned Dropdown components: introduced Dropdown.Contents, removed fixed height, updated context and item interfaces (optional properties, styles, onClick) for flexible styling.
.../components/Input/* Enhanced Input components: added className support and conditional label rendering; updated common input interface to provide additional styling options.
.../components/Modal/* Streamlined Modal logic: switched to a global modal hook, removed context/provider artifacts, explicitly defined children in ModalProps, and removed redundant wrappers.
.../features/discussion/ui/DiscussionForm/* Refactored discussion form dropdowns: added a new Trigger component for input handling, wrapped items within Dropdown.Contents and removed height properties.
.../features/user/api/*, .../features/user/model/* Expanded user API: added PATCH nickname update via patchNickname, new useNicknameMutation hook, and a Zod UserNicknameRequest schema and type.
.../hooks/useModal.ts Exported the ModalWithoutIsOpen type alias for external use.
.../layout/GlobalNavBar/* Refactored GlobalNavBar: moved inlined UserAvatar to its own component, updated styles with pointer cursor, and modified imports accordingly.
.../routes/__root.tsx, .../store/global/modal.ts Integrated GlobalModal: added GlobalModal in route rendering and introduced Jotai-based global modal state management.
.../utils/auth/index.ts, .../utils/fetch/index.ts Updated utilities: added a logout function for clearing access tokens and extended fetch methods to include a PATCH request handler.

Sequence Diagram(s)

sequenceDiagram
    participant User as User
    participant UA as UserAvatar
    participant DD as Dropdown
    participant GM as GlobalModal
    participant API as userApi.patchNickname

    User->>UA: Click avatar to open dropdown
    UA->>DD: Render dropdown with options
    User->>DD: Select "Change Nickname"
    DD->>GM: Open nickname modal
    User->>GM: Enter new nickname & click Save
    GM->>API: Send PATCH request with new nickname
    API-->>GM: Return updated user info
    GM->>UA: Update display and close modal
Loading

Possibly related PRs

Suggested labels

🛠️ BE

Suggested reviewers

  • kwon204
  • dioo1461

Poem

I hopped through code fields with a joyful bound,
Discovering changes where new fixes are found.
With modals and avatars now fresh and bright,
Dropdowns and inputs now dance in the light.
As a bunny coder, I cheer every tweak,
Celebrating our code with a happy little squeak!


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🔭 Outside diff range comments (1)
frontend/src/utils/auth/index.ts (1)

17-17: 🛠️ Refactor suggestion

Use ACCESS_TOKEN_KEY constant for consistency.

The hardcoded string 'accessToken' should be replaced with the ACCESS_TOKEN_KEY constant for consistency and maintainability.

-    localStorage.removeItem('accessToken');
+    localStorage.removeItem(ACCESS_TOKEN_KEY);
🧹 Nitpick comments (20)
frontend/src/components/Avatar/AvatarItem.tsx (1)

15-15: Consider moving the fallback URL to a constants file.

The hardcoded fallback URL could be moved to a shared constants file for better maintainability and reuse across components.

+// In src/constants/images.ts
+export const DEFAULT_AVATAR_URL = 'https://picsum.photos/id/200/200/200';

-  const fallbackSrc = 'https://picsum.photos/id/200/200/200';
+  const fallbackSrc = DEFAULT_AVATAR_URL;

Also applies to: 29-29

frontend/src/components/Avatar/index.tsx (1)

12-12: Enhance click handler accessibility.

While the click handler implementation is correct, consider these accessibility improvements:

  • Add keyboard support
  • Include ARIA attributes for dropdown trigger
  • Add visual focus indicator
 interface AvatarProps {
   size: Size;
   imageUrls: (string | null)[];
   onClick?: () => void;
   className?: string;
+  isOpen?: boolean; // For ARIA state
 }

-const Avatar = ({ size, imageUrls, onClick, className }: AvatarProps) => {
+const Avatar = ({ size, imageUrls, onClick, className, isOpen }: AvatarProps) => {
   // ...
   return (
     <div 
       className={clsx(avatarContainerStyle, className)} 
-      onClick={onClick}
+      onClick={onClick}
+      onKeyDown={(e) => e.key === 'Enter' && onClick?.()}
+      role="button"
+      tabIndex={0}
+      aria-expanded={isOpen}
+      aria-haspopup="true"
     >

Also applies to: 18-18, 23-23

frontend/src/layout/GlobalNavBar/index.css.ts (3)

26-27: Consider limiting pointer cursor to interactive elements.

Adding cursor: 'pointer' to the entire navbar container might be misleading as it suggests the whole navbar is clickable. Instead, consider applying the pointer cursor only to interactive elements within the navbar.

-    cursor: 'pointer',

52-54: Add max-height constraint for better mobile experience.

Consider adding a max-height constraint with overflow handling to prevent the modal from extending beyond the viewport on smaller screens.

 export const nicknameModalStyle = style({
   height: 'fit-content',
+  maxHeight: '80vh',
+  overflow: 'auto',
 });

60-62: Consider adding consistent padding on all sides.

For better visual balance, consider adding padding to all sides of the modal contents.

 export const nicknameModalContentsStyle = style({
-  paddingTop: vars.spacing[600],
+  padding: vars.spacing[600],
 });
frontend/src/features/discussion/ui/DiscussionConfirmCard/WarnModal.tsx (1)

11-38: Consider these improvements for better code clarity.

  1. Destructure specific props instead of using spread operator for better readability.
  2. Simplify the confirm button's onClick handler.

Consider this refactor:

-const WarnModal = ({ onConfirm, ...modalProps }: WarnModalProps) => (
+const WarnModal = ({ onConfirm, isOpen, onModalClose }: WarnModalProps) => (
   <Modal
     description={`참여자들의 캘린더에서 확정되었던 일정이 삭제됩니다.\n
       마감기한이 지났다면, 7일 연장해드릴게요!`}
-    isOpen={modalProps.isOpen}
+    isOpen={isOpen}
     subTitle='경고'
     title='일정 조율을 다시 진행하시겠습니까?'
     type='error'
   >
     <Modal.Footer className={modalFooterStyle}>
       <Button
-        onClick={modalProps.onModalClose}
+        onClick={onModalClose}
         size='xl'
         style='weak'
         variant='secondary'
       >
         취소
       </Button>
       <Button
-        onClick={() => onConfirm()}
+        onClick={onConfirm}
         size='xl'
         variant='re'
       >
         다시 진행하기
       </Button>
     </Modal.Footer>
   </Modal>
 );
frontend/src/components/Modal/Modal.stories.tsx (1)

72-72: Track TODO: Add tests for Jotai state integration.

The TODO comment indicates missing test coverage for the modal component with global Jotai state. This should be addressed to ensure proper testing of the modal's state management.

Would you like me to help create test cases for the modal component with Jotai state integration?

frontend/src/components/Modal/index.tsx (1)

52-52: Fix typo in color variable name.

There's a typo in the color variable name 'Netural' which should be 'Neutral'.

-      color={vars.color.Ref.Netural[800]}
+      color={vars.color.Ref.Neutral[800]}
-      color={type === 'default' ? vars.color.Ref.Netural[500] : vars.color.Ref.Netural[800]}
+      color={type === 'default' ? vars.color.Ref.Neutral[500] : vars.color.Ref.Neutral[800]}

Also applies to: 60-60

frontend/src/components/Dropdown/DropdownItem.tsx (1)

22-26: Consider combining the handlers for better maintainability.

The click handling could be simplified by combining the callbacks.

-  const handleClick = () => {
-    onChange?.(value);
-    onClick?.();
-    setIsOpen(false);
-  };
+  const handleClick = () => {
+    Promise.resolve()
+      .then(() => onChange?.(value))
+      .then(() => onClick?.())
+      .finally(() => setIsOpen(false));
+  };
frontend/src/components/Dropdown/Dropdown.stories.tsx (1)

37-43: Consider adding more story variants.

The current story demonstrates basic usage well. Consider adding variants that showcase:

  • Custom styling through className and style props
  • Different width/height configurations
  • Usage without selection tracking (new optional props)
frontend/src/features/discussion/ui/DiscussionForm/MeetingDurationDropdown.tsx (2)

11-27: Consider making Trigger component more reusable.

The Trigger component is tightly coupled to the form context and specific input requirements. Consider making it more generic by:

  1. Accepting props for label and validation
  2. Making the form context optional
-const Trigger = ({ name }: { name: keyof DiscussionRequest }) => {
+interface TriggerProps {
+  name: keyof DiscussionRequest;
+  label?: string;
+  required?: boolean;
+  useFormContext?: boolean;
+}
+
+const Trigger = ({ name, label = '미팅 시간', required = true, useFormContext = true }: TriggerProps) => {
+  const formContext = useFormContext();
+  const { formState, errors, isValid, handleUpdateField } = useFormContext ? formContext : {
+    formState: {},
+    errors: () => '',
+    isValid: () => true,
+    handleUpdateField: () => {},
+  };

31-37: Consider adding error boundary for time validation.

The validation logic could throw if timeRangeStart or timeRangeEnd are undefined.

   const validateDuration = () => {
+    if (!formState.timeRangeStart || !formState.timeRangeEnd) {
+      return '시간 범위를 먼저 설정해주세요.';
+    }
     const startTime = formatTimeStringToNumber(formState.timeRangeStart);
     const endTime = formatTimeStringToNumber(formState.timeRangeEnd);
     if (endTime - startTime < formState.duration) return '회의 소요시간은 논의 범위를 초과할 수 없습니다.';
     return '';
   };
frontend/src/features/discussion/ui/DiscussionForm/MeetingTimeDropdowns.tsx (2)

18-23: Consider enhancing time range validation.

The current validation only checks if end time is after start time. Consider adding:

  1. Business hours validation
  2. Minimum time difference check
   const validateTimeRange = () => {
+    const MIN_TIME_DIFFERENCE = 30; // minutes
     const start = formatTimeStringToNumber(formState.timeRangeStart);
     const end = formatTimeStringToNumber(formState.timeRangeEnd);
+    
+    if (start < 9 * 60 || end > 18 * 60) {
+      return '업무 시간(9:00-18:00) 내에서 선택해주세요.';
+    }
+    
     if (start >= end) return '종료 시간은 시작 시간보다 빠를 수 없습니다.';
+    
+    if (end - start < MIN_TIME_DIFFERENCE) {
+      return `최소 ${MIN_TIME_DIFFERENCE}분 이상 선택해주세요.`;
+    }
+    
     return '';
   };

58-64: Consider memoizing dropdown items.

The time options array is being recreated on every render. Consider memoizing it for better performance.

+const timeOptions = MINUTES_HALF(24, 0).map((minute) => ({
+  value: formatNumberToTimeString(minute),
+  label: formatMinutesToTimeString(minute),
+  key: minute,
+}));

 <Dropdown.Contents height={168}>
-  {MINUTES_HALF(24, 0).map((minute) => (
+  {timeOptions.map(({ key, value, label }) => (
     <Dropdown.Item
-      key={minute}
-      value={formatNumberToTimeString(minute)}
+      key={key}
+      value={value}
     >
-      {formatMinutesToTimeString(minute)}
+      {label}
     </Dropdown.Item>
   ))}
 </Dropdown.Contents>
frontend/src/features/user/model/index.ts (1)

15-17: Consider adding validation rules for the name field.

The current schema only validates that the name is a string. Consider adding rules for:

  • Minimum and maximum length
  • Character set restrictions (if any)
  • Whitespace handling
 export const UserNicknameRequest = z.object({
-  name: z.string(),
+  name: z.string()
+    .min(2, 'Nickname must be at least 2 characters')
+    .max(30, 'Nickname cannot exceed 30 characters')
+    .trim()
+    .regex(/^[a-zA-Z0-9\s_-]*$/, 'Only alphanumeric characters, spaces, underscores, and hyphens are allowed'),
 });
frontend/src/features/user/api/mutations.ts (1)

9-21: Enhance mutation hook with error handling and loading state.

The hook should handle errors and expose loading state for better UX. Consider:

  1. Adding error handling callback
  2. Exposing isLoading state
  3. Adding error retry configuration
 export const useNicknameMutation = () => {
   const queryClient = useQueryClient();
-  const { mutate } = useMutation({
+  const { mutate, isLoading, error } = useMutation({
     mutationFn: ({ name }: UserNicknameRequest ) => userApi.patchNickname({ name }),
     onSuccess: () => {
       queryClient.invalidateQueries({
         queryKey: userInfoQueryKey(getAccessToken()),
       });
     },
+    onError: (error) => {
+      console.error('Failed to update nickname:', error);
+    },
+    retry: 2,
   });

-  return { mutate };
+  return { mutate, isLoading, error };
 };
frontend/src/utils/auth/index.ts (1)

30-32: Consider clearing all auth-related data on logout.

The logout function only clears the access token. Consider:

  1. Clearing any other auth-related data in localStorage
  2. Invalidating relevant queries
  3. Redirecting to login page
 export const logout = () => {
   localStorage.removeItem(ACCESS_TOKEN_KEY);
+  // Clear other auth-related data if any
+  queryClient.clear(); // Clear all queries
+  window.location.href = '/login'; // Redirect to login
 };
frontend/src/utils/fetch/index.ts (1)

81-95: Update documentation to include PATCH method.

The documentation comment needs to be updated to include the newly added PATCH method in the list of supported HTTP methods.

 * HTTP 요청을 수행하기 위한 헬퍼 메서드를 제공합니다.
 *
 * 이 객체는 일반적인 API 작업을 수행하기 위해 HTTP 메서드를 캡슐화합니다.
-* GET, POST, PUT, DELETE 요청을 헬퍼 함수로 지원하며,
+* GET, POST, PUT, PATCH, DELETE 요청을 헬퍼 함수로 지원하며,
 * 각 메서드는 프로미스를 반환합니다.
frontend/src/layout/GlobalNavBar/UserAvatar.tsx (2)

23-53: Consider form validation for nickname changes.

The nickname change form lacks input validation. Consider adding:

  • Minimum/maximum length validation
  • Special character restrictions
  • Empty input prevention
 const ModalContents = ({ name }: { name?: string }) => {
   const { mutate } = useNicknameMutation();
   const { onModalClose } = useGlobalModal();
   const { valuesRef, handleChange } = useFormRef({
     nickname: '',
   });
 
+  const isValidNickname = (nickname: string) => {
+    return nickname.trim().length >= 2 && nickname.trim().length <= 20;
+  };
 
   const handleClickChangeNickname = () => {
+    if (!isValidNickname(valuesRef.current.nickname)) {
+      return;
+    }
     mutate({ name: valuesRef.current.nickname });
     onModalClose();
   };

60-65: Consider adding loading state during logout.

The logout process might benefit from a loading state to prevent multiple clicks and provide better user feedback.

+  const [isLoggingOut, setIsLoggingOut] = useState(false);

   const handleClickLogout = () => {
+    if (isLoggingOut) return;
+    setIsLoggingOut(true);
     logout();
     navigate({
       to: '/login',
     });
   };
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2a2d236 and 8e47e5f.

📒 Files selected for processing (30)
  • frontend/src/components/Avatar/AvatarItem.tsx (2 hunks)
  • frontend/src/components/Avatar/index.tsx (1 hunks)
  • frontend/src/components/Dropdown/Dropdown.stories.tsx (1 hunks)
  • frontend/src/components/Dropdown/DropdownContents.tsx (1 hunks)
  • frontend/src/components/Dropdown/DropdownContext.ts (1 hunks)
  • frontend/src/components/Dropdown/DropdownItem.tsx (1 hunks)
  • frontend/src/components/Dropdown/index.tsx (2 hunks)
  • frontend/src/components/Input/MultiInput.tsx (2 hunks)
  • frontend/src/components/Input/SingleInput.tsx (2 hunks)
  • frontend/src/components/Input/index.tsx (1 hunks)
  • frontend/src/components/Modal/GlobalModal.tsx (1 hunks)
  • frontend/src/components/Modal/Modal.stories.tsx (1 hunks)
  • frontend/src/components/Modal/ModalContext.ts (0 hunks)
  • frontend/src/components/Modal/ModalProvider.tsx (0 hunks)
  • frontend/src/components/Modal/index.tsx (2 hunks)
  • frontend/src/features/discussion/ui/DiscussionConfirmCard/WarnModal.tsx (1 hunks)
  • frontend/src/features/discussion/ui/DiscussionForm/MeetingDurationDropdown.tsx (2 hunks)
  • frontend/src/features/discussion/ui/DiscussionForm/MeetingMethodDropdown.tsx (1 hunks)
  • frontend/src/features/discussion/ui/DiscussionForm/MeetingTimeDropdowns.tsx (1 hunks)
  • frontend/src/features/user/api/index.ts (1 hunks)
  • frontend/src/features/user/api/mutations.ts (1 hunks)
  • frontend/src/features/user/model/index.ts (1 hunks)
  • frontend/src/hooks/useModal.ts (1 hunks)
  • frontend/src/layout/GlobalNavBar/UserAvatar.tsx (1 hunks)
  • frontend/src/layout/GlobalNavBar/index.css.ts (2 hunks)
  • frontend/src/layout/GlobalNavBar/index.tsx (1 hunks)
  • frontend/src/routes/__root.tsx (2 hunks)
  • frontend/src/store/global/modal.ts (1 hunks)
  • frontend/src/utils/auth/index.ts (1 hunks)
  • frontend/src/utils/fetch/index.ts (2 hunks)
💤 Files with no reviewable changes (2)
  • frontend/src/components/Modal/ModalContext.ts
  • frontend/src/components/Modal/ModalProvider.tsx
✅ Files skipped from review due to trivial changes (1)
  • frontend/src/features/discussion/ui/DiscussionForm/MeetingMethodDropdown.tsx
🧰 Additional context used
🧠 Learnings (1)
frontend/src/components/Input/MultiInput.tsx (1)
Learnt from: hamo-o
PR: softeer5th/Team4-enDolphin#100
File: frontend/src/components/Input/MultiInput.tsx:19-22
Timestamp: 2025-02-06T02:51:01.268Z
Learning: In the Input component, the `borderPlacement` prop should be optional with a default value of 'inputField' to maintain backward compatibility and improve developer experience.
🔇 Additional comments (24)
frontend/src/components/Avatar/AvatarItem.tsx (1)

7-7: LGTM! Type change improves null safety.

The update to allow null values for the src prop aligns well with the avatar dropdown feature, properly handling cases where users might not have profile images.

frontend/src/components/Avatar/index.tsx (1)

11-11: LGTM! Type change maintains consistency.

The update to allow null values in imageUrls array maintains consistency with AvatarItem changes.

frontend/src/layout/GlobalNavBar/index.css.ts (1)

56-58: LGTM!

Good use of theme spacing variables for consistent padding.

frontend/src/components/Input/index.tsx (2)

15-15: LGTM! Adding className prop enhances styling flexibility.

The addition of an optional className prop follows React conventions and allows for better styling customization.


8-8:

Details

❓ Verification inconclusive

Verify the impact of making label optional.

Making the label property optional is a breaking change that could affect existing components. Please ensure all consumers of this interface are updated accordingly.

Run the following script to find all components using the label prop:


🏁 Script executed:

#!/bin/bash
# Description: Find all components using the label prop from CommonInputProps.

# Search for components using the label prop
rg -A 5 'label[^a-zA-Z].*CommonInputProps'

Length of output: 44


Action Required: Manually Verify Usage of the Optional label Property

The change to make label optional in frontend/src/components/Input/index.tsx (line 8) appears isolated, and our automated search did not return any direct usage of label tied to CommonInputProps. However, the empty output prevents us from conclusively ruling out any impact on consumers.

  • Location: frontend/src/components/Input/index.tsx (line 8)
  • Concern: Confirm that no consumer of CommonInputProps relies on label being required.
  • Next Step: Please manually verify and review the affected components to ensure that making label optional does not introduce breaking behavior.
frontend/src/components/Input/SingleInput.tsx (2)

3-4: LGTM! Proper utility import for class name handling.

The clsx utility is correctly imported for handling conditional class names.


29-30: LGTM! Proper implementation of className and conditional label rendering.

The changes correctly:

  • Use clsx for merging class names
  • Add conditional rendering for the optional label
frontend/src/components/Input/MultiInput.tsx (1)

34-34: LGTM! Consistent className implementation across input components.

The className prop is correctly added and implemented using clsx, maintaining consistency with SingleInput.

Also applies to: 46-46

frontend/src/store/global/modal.ts (2)

5-6: LGTM! Well-structured atoms for modal state.

The atoms are correctly typed and initialized with appropriate default values.


8-24: LGTM! Clean and well-implemented modal management hook.

The hook provides a comprehensive interface for modal management with proper state handling.

frontend/src/components/Modal/GlobalModal.tsx (1)

9-25: LGTM! Well-implemented global modal component.

The component follows React best practices with proper Portal usage and click-outside handling.

frontend/src/hooks/useModal.ts (1)

5-5: LGTM! Improved type reusability.

Making ModalWithoutIsOpen exportable enhances its reusability across the application.

frontend/src/routes/__root.tsx (1)

8-8: LGTM! Good placement of GlobalModal for application-wide accessibility.

The GlobalModal is correctly placed at the root level and before the Outlet component, ensuring it's accessible throughout the application and renders above all route content.

Also applies to: 30-30

frontend/src/components/Modal/index.tsx (1)

2-2: LGTM! Improved type clarity with explicit children prop.

The change from extending PropsWithChildren to explicitly defining the children prop improves type clarity and maintainability.

Also applies to: 14-22

frontend/src/components/Dropdown/DropdownContext.ts (1)

4-7: LGTM! Making selectedValue and onChange optional improves component flexibility.

The changes align well with the avatar dropdown requirements where selection tracking might not be needed. The documentation remains accurate and helpful.

frontend/src/components/Dropdown/DropdownContents.tsx (1)

7-24: Well-structured implementation with good flexibility!

The component follows React best practices:

  • Uses semantic HTML (<ul>)
  • Provides flexible styling options
  • Has sensible defaults
  • Properly types props with TypeScript
frontend/src/components/Dropdown/DropdownItem.tsx (1)

11-16: LGTM! Good additions for styling and click handling flexibility.

The new props enhance the component's customization capabilities while maintaining type safety.

frontend/src/components/Dropdown/index.tsx (3)

14-16: LGTM! Props interface changes improve component flexibility.

Making onChange and selectedValue optional allows the component to be used for both controlled and uncontrolled scenarios.


43-43: LGTM! Style handling is well implemented.

The spread operator correctly combines width with custom styles, maintaining good flexibility while preserving the width prop's priority.

Also applies to: 48-48


54-54: LGTM! Good component composition.

Exposing DropdownContents as a subcomponent follows the compound component pattern, improving the API's usability and maintainability.

frontend/src/features/discussion/ui/DiscussionForm/MeetingDurationDropdown.tsx (1)

46-53: LGTM! Well-structured dropdown content.

Good use of the new Dropdown.Contents component with appropriate height and clear item structure.

frontend/src/utils/fetch/index.ts (1)

9-9: LGTM! PATCH method implementation.

The PATCH method is correctly implemented following the same pattern as other HTTP methods, maintaining consistency in the codebase.

Also applies to: 103-104

frontend/src/layout/GlobalNavBar/UserAvatar.tsx (1)

102-102:

Details

❓ Verification inconclusive

Verify image URL handling for null case.

The Avatar component receives a null image URL when data is loading. Ensure the component properly handles this case with a fallback image.


🏁 Script executed:

#!/bin/bash
# Search for Avatar component implementation to verify null handling
ast-grep --pattern 'const Avatar = ({ imageUrls $_ }) => { $$$ }'

Length of output: 67


Action: Validate Fallback Handling in Avatar Component

  • In frontend/src/layout/GlobalNavBar/UserAvatar.tsx at line 102, the component passes null to the imageUrls prop when data is loading.
  • The current search for the Avatar implementation produced no clear evidence that a fallback image is applied for a null URL.
  • Next Step: Please manually verify whether the Avatar component (or its underlying library implementation) correctly handles a null value by applying a fallback image. If it does not, consider updating the code or the Avatar component to safely render a default image.
frontend/src/layout/GlobalNavBar/index.tsx (1)

10-11: LGTM! Clean component separation.

The refactoring improves modularity by moving the UserAvatar logic to a dedicated component while maintaining the conditional rendering logic based on login status.

Also applies to: 33-40

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
frontend/src/components/Modal/GlobalModal.tsx (2)

10-13: Consider adding type safety for the modal object.

The refactoring to use useGlobalModal improves the architecture, but consider adding type annotations to make the shape of the modal object explicit.

+ type ModalContent = {
+   title?: string;
+   children: React.ReactNode;
+   // Add other modal properties as needed
+ };
+
- export const GlobalModal = () => {
+ export const GlobalModal = (): JSX.Element => {
-   const { isOpen, onModalClose, modal } = useGlobalModal();
+   const { isOpen, onModalClose, modal } = useGlobalModal<ModalContent>();

20-31: Enhance backdrop semantics and prop safety.

Consider these improvements:

  1. Add ARIA attributes to the backdrop for better accessibility
  2. Be explicit about which props are spread to avoid potential prop pollution
  if (!modal || !isOpen) return null;
  return createPortal(
    <>
      <Modal
        isOpen={isOpen}
        ref={ref}
-       {...modal}
+       title={modal.title}
+       children={modal.children}
+       // Explicitly list other needed props
      />
-     <div className={backdropStyle} />
+     <div 
+       className={backdropStyle}
+       role="presentation"
+       aria-hidden="true"
+     />
    </>
    , MODAL_ROOT);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8e47e5f and a8a2c77.

📒 Files selected for processing (1)
  • frontend/src/components/Modal/GlobalModal.tsx (1 hunks)
🔇 Additional comments (1)
frontend/src/components/Modal/GlobalModal.tsx (1)

1-5: LGTM! Clean and well-organized imports.

The imports are properly structured and include all necessary dependencies for the modal functionality.

Copy link
Contributor

@dioo1461 dioo1461 left a comment

Choose a reason for hiding this comment

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

잘 되네요 고생하셨습니다!

UX적으로 사소한 문제가 있는데, 닉네임 변경 창 밖에서 mouseUp 이벤트가 발생하면 모달 창이 꺼져버립니다,,
재현) input field에서 드래그 시작, modal 창 밖에서 드래그 종료

image

@hamo-o hamo-o merged commit 2013813 into dev Feb 24, 2025
1 check passed
@hamo-o hamo-o deleted the feature/fe/profile branch February 24, 2025 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🖥️ FE Frontend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] 헤더 아바타 드롭다운 구현

3 participants