refactor: unused component cleanup - Must be merged after #3242#3258
refactor: unused component cleanup - Must be merged after #3242#3258ogzhanolguncu merged 12 commits intomainfrom
Conversation
ogzhanolguncu
requested review from
chronark,
mcstepp and
perkinsjr
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
📝 Walkthrough## Walkthrough
This change refactors form components and UI primitives across the dashboard app to remove custom local UI implementations (such as form wrappers, input fields, checkboxes, tooltips, and related utilities) in favor of standardized components from the `@unkey/ui` library. Numerous local UI component files are deleted, and affected feature components are updated to use the new imports and simplified form integration patterns.
## Changes
| Files/Paths | Change Summary |
|-------------------------------------------------------------------------------------------------|----------------|
| `apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-bytes.tsx`,<br>`.../default-prefix.tsx`,<br>`.../update-api-name.tsx`,<br>`.../update-ip-whitelist.tsx`,<br>`.../settings/root-keys/[keyId]/update-root-key-name.tsx`,<br>`.../settings/team/invite.tsx`,<br>`.../new/create-api.tsx`,<br>`.../new/create-workspace.tsx` | Refactored form handling to remove custom form components and use `react-hook-form`'s `Controller` and `FormInput` from `@unkey/ui`, simplifying form structure and validation logic. |
| `apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/rbac/permissions.tsx`,<br>`.../authorization/roles/[roleId]/permission-toggle.tsx`,<br>`.../settings/root-keys/[keyId]/permissions/permission_toggle.tsx`,<br>`.../settings/root-keys/new/client.tsx` | Changed `Checkbox` import from local UI to `@unkey/ui`. |
| `apps/dashboard/app/(app)/authorization/roles/[roleId]/tree.tsx`,<br>`.../components/dashboard/root-key-table/index.tsx`,<br>`.../components/logs/details/request-response-details.tsx`,<br>`.../components/navigation/sidebar/team-switcher.tsx` | Replaced generic tooltip components with domain-specific `RatelimitOverviewTooltip` for permission and workspace tooltips. |
| `apps/dashboard/app/(app)/settings/vercel/client.tsx`,<br>`.../components/ui/sidebar.tsx`,<br>`.../components/logs/llm-search/components/search-example-tooltip.tsx` | Changed `Tooltip`, `Input`, and `Select` imports to `@unkey/ui`. |
| `apps/dashboard/app/auth/sign-in/email-signin.tsx`,<br>`.../sign-up/email-signup.tsx` | Replaced local `Input` with `FormInput` from `@unkey/ui` and adjusted form layout and styling. |
| `apps/dashboard/app/layout.tsx` | Removed `TooltipProvider` wrapper from layout. |
| `apps/dashboard/components/logs/checkbox/filter-checkbox.tsx`,<br>`.../filter-operator-input/index.tsx` | Changed `Checkbox` and `Textarea` imports to `@unkey/ui` and simplified textarea styling. |
| `apps/dashboard/components/dashboard/feedback-component.tsx` | Refactored feedback dialog to use `DialogContainer`, simplified form structure, and improved error handling. |
| `apps/dashboard/components/dashboard/api-key-table/index.tsx`,<br>`.../table.tsx`,<br>`.../create-key-button.tsx`,<br>`.../root-key-table/table.tsx`,<br>`.../ui/accordion.tsx`,<br>`.../ui/alert-dialog.tsx`,<br>`.../ui/back-button.tsx`,<br>`.../ui/checkbox.tsx`,<br>`.../ui/form.tsx`,<br>`.../ui/group-button.tsx`,<br>`.../ui/input.tsx`,<br>`.../ui/pagination.tsx`,<br>`.../ui/progress.tsx`,<br>`.../ui/skeleton.tsx`,<br>`.../ui/textarea.tsx`,<br>`.../ui/tooltip.tsx`,<br>`.../components/banner.tsx`,<br>`.../app/(app)/ratelimits/loading.tsx`,<br>`.../app/(app)/settings/loading.tsx` | Deleted local UI component implementations and feature components that are now replaced by standardized components from `@unkey/ui` or no longer needed. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant FormComponent
participant react-hook-form
participant @unkey/ui
User->>FormComponent: Interact with form (input/change/submit)
FormComponent->>@unkey/ui: Render FormInput, Checkbox, etc.
FormComponent->>react-hook-form: Register fields via Controller
react-hook-form-->>FormComponent: Provide field state (isDirty, isValid, errors)
User->>FormComponent: Submit form
FormComponent->>react-hook-form: handleSubmit(onSubmit)
react-hook-form->>FormComponent: Validate and pass data
FormComponent->>API: (if applicable) Submit dataPossibly related PRs
Suggested labels
Suggested reviewers
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
ogzhanolguncu
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
apps/dashboard/components/dashboard/feedback-component.tsx (1)
52-61: 🛠️ Refactor suggestionReset the form after a successful submission
The dialog closes, but the component stays mounted, meaning the next time the user opens it the previous input is still there.
Callingreset()restores the default values and avoids this confusion.- onSuccess: () => { - setOpen(false); + onSuccess: () => { + reset(); // clear the form + setOpen(false);
🧹 Nitpick comments (5)
apps/dashboard/app/new/create-workspace.tsx (1)
113-143: Form implementation is successfully simplifiedThe form implementation has been properly streamlined by:
- Using the Controller pattern with FormInput
- Leveraging react-hook-form's handleSubmit
- Properly managing button states with form validation
Consider adding a placeholder to the FormInput for better user guidance.
<FormInput {...field} error={errors.name?.message} description="What should your workspace be called?" + placeholder="My Awesome Workspace" />apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-bytes.tsx (1)
65-105: Form layout simplified correctlyThe refactored form layout with SettingCard and Controller pattern is cleaner. The number input handling is properly implemented with regex pattern to only allow digits.
One edge case to consider: the current onChange handler may convert empty input to 0, which could be unexpected for users.
<FormInput {...field} className="w-[16.5rem]" autoComplete="off" type="text" - onChange={(e) => field.onChange(Number(e.target.value.replace(/\D/g, "")))} + onChange={(e) => { + const value = e.target.value.replace(/\D/g, ""); + field.onChange(value ? Number(value) : undefined); + }} + placeholder="8-255" />apps/dashboard/app/(app)/settings/team/invite.tsx (1)
112-144: Form implementation properly modernizedThe form is correctly refactored using both direct register for simple fields and Controller for complex inputs. Error handling is properly implemented.
For consistency with the FormInput component, consider adding an error state to the Select component when validation fails.
<Select onValueChange={field.onChange} value={field.value}> - <SelectTrigger className="h-9"> + <SelectTrigger className={`h-9 ${errors.role ? "border-error-7 focus:border-error-8" : ""}`}> <SelectValue placeholder="Select a role" /> </SelectTrigger>apps/dashboard/components/dashboard/feedback-component.tsx (2)
63-69: Remove the redundanttry/catchblock
mutateAsyncalready triggers theonErrorcallback and returns a rejected promise, so thistry/catchonly duplicates console output without adding error handling value.- const onSubmitForm = async (values: FormValues) => { - try { - await create.mutateAsync(values); - } catch (error) { - console.error("Form submission error:", error); - } - }; + const onSubmitForm = (values: FormValues) => create.mutateAsync(values);
80-86: Disable the button when the form is invalidWith
isValid(see earlier comment) the button can be disabled until all validation passes, preventing an avoidable round-trip.- disabled={isSubmitting || create.isLoading} - loading={isSubmitting || create.isLoading} + disabled={!isValid || isSubmitting || create.isLoading} + loading={isSubmitting || create.isLoading}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (44)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/permission-list.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-bytes.tsx(3 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-prefix.tsx(3 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/components/update-api-name.tsx(4 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/components/update-ip-whitelist.tsx(5 hunks)apps/dashboard/app/(app)/authorization/roles/[roleId]/permission-toggle.tsx(1 hunks)apps/dashboard/app/(app)/authorization/roles/[roleId]/tree.tsx(2 hunks)apps/dashboard/app/(app)/ratelimits/loading.tsx(0 hunks)apps/dashboard/app/(app)/settings/loading.tsx(0 hunks)apps/dashboard/app/(app)/settings/root-keys/[keyId]/permissions/permission_toggle.tsx(1 hunks)apps/dashboard/app/(app)/settings/root-keys/[keyId]/update-root-key-name.tsx(3 hunks)apps/dashboard/app/(app)/settings/root-keys/new/client.tsx(1 hunks)apps/dashboard/app/(app)/settings/team/invite.tsx(3 hunks)apps/dashboard/app/(app)/settings/vercel/client.tsx(1 hunks)apps/dashboard/app/auth/sign-in/email-signin.tsx(2 hunks)apps/dashboard/app/auth/sign-up/email-signup.tsx(2 hunks)apps/dashboard/app/layout.tsx(1 hunks)apps/dashboard/app/new/create-api.tsx(4 hunks)apps/dashboard/app/new/create-workspace.tsx(2 hunks)apps/dashboard/components/banner.tsx(0 hunks)apps/dashboard/components/dashboard/api-key-table/index.tsx(0 hunks)apps/dashboard/components/dashboard/api-key-table/table.tsx(0 hunks)apps/dashboard/components/dashboard/create-key-button.tsx(0 hunks)apps/dashboard/components/dashboard/feedback-component.tsx(3 hunks)apps/dashboard/components/dashboard/root-key-table/index.tsx(3 hunks)apps/dashboard/components/dashboard/root-key-table/table.tsx(1 hunks)apps/dashboard/components/logs/checkbox/filter-checkbox.tsx(1 hunks)apps/dashboard/components/logs/details/request-response-details.tsx(2 hunks)apps/dashboard/components/logs/filter-operator-input/index.tsx(2 hunks)apps/dashboard/components/logs/llm-search/components/search-example-tooltip.tsx(1 hunks)apps/dashboard/components/navigation/sidebar/team-switcher.tsx(2 hunks)apps/dashboard/components/ui/accordion.tsx(0 hunks)apps/dashboard/components/ui/alert-dialog.tsx(0 hunks)apps/dashboard/components/ui/back-button.tsx(0 hunks)apps/dashboard/components/ui/checkbox.tsx(0 hunks)apps/dashboard/components/ui/form.tsx(0 hunks)apps/dashboard/components/ui/group-button.tsx(0 hunks)apps/dashboard/components/ui/input.tsx(0 hunks)apps/dashboard/components/ui/pagination.tsx(0 hunks)apps/dashboard/components/ui/progress.tsx(0 hunks)apps/dashboard/components/ui/sidebar.tsx(1 hunks)apps/dashboard/components/ui/skeleton.tsx(0 hunks)apps/dashboard/components/ui/textarea.tsx(0 hunks)apps/dashboard/components/ui/tooltip.tsx(0 hunks)
💤 Files with no reviewable changes (18)
- apps/dashboard/components/ui/back-button.tsx
- apps/dashboard/components/ui/checkbox.tsx
- apps/dashboard/components/ui/skeleton.tsx
- apps/dashboard/components/ui/textarea.tsx
- apps/dashboard/components/dashboard/api-key-table/table.tsx
- apps/dashboard/app/(app)/settings/loading.tsx
- apps/dashboard/components/banner.tsx
- apps/dashboard/components/ui/tooltip.tsx
- apps/dashboard/components/dashboard/create-key-button.tsx
- apps/dashboard/components/ui/alert-dialog.tsx
- apps/dashboard/components/ui/progress.tsx
- apps/dashboard/app/(app)/ratelimits/loading.tsx
- apps/dashboard/components/ui/group-button.tsx
- apps/dashboard/components/ui/pagination.tsx
- apps/dashboard/components/ui/accordion.tsx
- apps/dashboard/components/ui/input.tsx
- apps/dashboard/components/dashboard/api-key-table/index.tsx
- apps/dashboard/components/ui/form.tsx
🧰 Additional context used
🧠 Learnings (1)
apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-prefix.tsx (1)
Learnt from: chronark
PR: unkeyed/unkey#2146
File: apps/dashboard/app/(app)/apis/[apiId]/settings/default-prefix.tsx:74-75
Timestamp: 2024-10-04T17:27:09.821Z
Learning: In `apps/dashboard/app/(app)/apis/[apiId]/settings/default-prefix.tsx`, the hidden `<input>` elements for `workspaceId` and `keyAuthId` work correctly without being registered with React Hook Form.
🧬 Code Graph Analysis (7)
apps/dashboard/app/layout.tsx (2)
apps/dashboard/components/dashboard/command-menu.tsx (1)
CommandMenu(16-49)apps/dashboard/components/dashboard/feedback-component.tsx (1)
Feedback(36-177)
apps/dashboard/components/navigation/sidebar/team-switcher.tsx (1)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/components/ratelimit-overview-tooltip.tsx (1)
RatelimitOverviewTooltip(10-37)
apps/dashboard/components/logs/details/request-response-details.tsx (1)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/components/ratelimit-overview-tooltip.tsx (1)
RatelimitOverviewTooltip(10-37)
apps/dashboard/app/(app)/authorization/roles/[roleId]/tree.tsx (3)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/components/ratelimit-overview-tooltip.tsx (1)
RatelimitOverviewTooltip(10-37)apps/dashboard/components/dashboard/copy-button.tsx (1)
CopyButton(15-52)apps/dashboard/app/(app)/authorization/roles/[roleId]/permission-toggle.tsx (1)
PermissionToggle(15-83)
apps/dashboard/app/auth/sign-in/email-signin.tsx (1)
internal/ui/src/components/form/form-input.tsx (1)
FormInput(18-89)
apps/dashboard/app/auth/sign-up/email-signup.tsx (2)
internal/ui/src/components/form/form-input.tsx (1)
FormInput(18-89)apps/dashboard/app/(app)/apis/loading.tsx (1)
Loading(5-156)
apps/dashboard/app/(app)/settings/team/invite.tsx (3)
internal/icons/src/icons/plus.tsx (1)
Plus(16-52)apps/dashboard/components/dialog-container/dialog-container.tsx (1)
DialogContainer(18-48)internal/ui/src/components/form/form-input.tsx (1)
FormInput(18-89)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Test Packages / Test ./packages/cache
- GitHub Check: Test Packages / Test ./packages/nextjs
- GitHub Check: Test Packages / Test ./packages/hono
- GitHub Check: Test Packages / Test ./packages/api
- GitHub Check: Test Packages / Test ./internal/keys
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: Test Packages / Test ./internal/encryption
- GitHub Check: autofix
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (57)
apps/dashboard/app/(app)/authorization/roles/[roleId]/permission-toggle.tsx (1)
5-5:Details
❓ Verification inconclusive
Standardize Checkbox import from external UI library
ImportingCheckboxfrom@unkey/uialigns with the broader UI cleanup. Verify that theCheckboxAPI (props likecheckedandonClick) matches the previous implementation to avoid regressions.Run the following to ensure no leftover local imports:
🏁 Script executed:
rg 'from "@/components/ui/checkbox"' -nLength of output: 41
Standardize external Checkbox import
No remaining local imports ofCheckboxdetected. Please confirm that the newCheckboxfrom@unkey/uipreserves the previous API (props such aschecked,onClick, etc.) to avoid regressions.apps/dashboard/components/logs/llm-search/components/search-example-tooltip.tsx (1)
2-2: Replace local Tooltip imports with external library
Swapping inTooltip,TooltipContent,TooltipProvider, andTooltipTriggerfrom@unkey/uistandardizes the tooltip primitives. Confirm that these components support the same props (delayDuration,asChild, etc.) and styling behavior as the removed local implementation.apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/permission-list.tsx (1)
4-4: Migrate Checkbox to external UI library
This replaces the deleted localCheckboxwith the one from@unkey/ui. Please verify that it fully supports thecheckedandonCheckedChangeprops used below, and that there are no breaking API changes.apps/dashboard/app/(app)/settings/root-keys/[keyId]/permissions/permission_toggle.tsx (1)
6-6: Ensure correct event handler for Checkbox
@unkey/ui’sCheckboxmay expect anonCheckedChangecallback instead ofonClick. Please verify the component’s API and update the handler prop to match (e.g., usingonCheckedChange={(val) => …}) to ensure toggles work as intended.apps/dashboard/components/dashboard/root-key-table/table.tsx (1)
42-42: Standardize Button and Input imports
Replacing localInputandButtonwith@unkey/uicomponents improves consistency. Confirm that the new components’ APIs (especiallyvalue,onChange,disabled, andclassName) align with their previous behavior to prevent regressions in filtering and dialog actions.apps/dashboard/components/ui/sidebar.tsx (1)
17-17: Good refactoring to use centralized UI components.The change to import tooltip components from
@unkey/uiinstead of a local implementation aligns with the PR's objective of cleaning up unused components and standardizing UI imports.apps/dashboard/components/logs/checkbox/filter-checkbox.tsx (1)
2-2: Appropriate consolidation of UI component imports.Importing
ButtonandCheckboxfrom@unkey/uiinstead of local implementations supports the codebase cleanup objective while maintaining the same functionality.apps/dashboard/app/layout.tsx (1)
62-66: Simplified component hierarchy by removing TooltipProvider.The removal of TooltipProvider wrapper around children, CommandMenu, and Feedback is consistent with the refactoring approach to move away from generic tooltip providers in favor of more specific tooltip implementations imported from
@unkey/ui.apps/dashboard/app/(app)/settings/root-keys/new/client.tsx (1)
7-7: Consolidated UI imports from centralized package.Replacing separate imports of UI components with a single import from
@unkey/uiimproves code maintainability and supports the goal of removing unused local component implementations.apps/dashboard/components/logs/filter-operator-input/index.tsx (2)
2-2: Good move on consolidating UI component imports.Importing
Textareafrom@unkey/uialigns with the overall refactoring effort to standardize UI components.
141-141: Clean simplification of textarea styling.Simplifying the styling to just
h-20makes the code more maintainable by leveraging the default styles from the@unkey/uipackage.apps/dashboard/app/(app)/settings/vercel/client.tsx (2)
23-33: Good consolidation of UI component imports.Consolidating imports from
@unkey/uipackage improves maintainability and consistency across the codebase.
1-4: Verify if this file should be included in cleanup.This file is marked as "Deprecated with new auth". Consider if it's worth including in the component cleanup effort if it will be removed in the near future.
apps/dashboard/components/dashboard/root-key-table/index.tsx (3)
3-3: Good addition of specialized tooltip component.Adding
RatelimitOverviewTooltipimport to replace the generic tooltip implementation improves consistency.
24-24: Good consolidation of UI component imports.Importing
Checkboxfrom@unkey/uipackage aligns with the refactoring effort to standardize components.
91-101: Improved tooltip implementation.Replacing the generic tooltip with
RatelimitOverviewTooltipimproves UI consistency and simplifies the tooltip configuration through props.apps/dashboard/components/navigation/sidebar/team-switcher.tsx (2)
2-2: Good addition of specialized tooltip component.Adding
RatelimitOverviewTooltipimport to replace the generic tooltip implementation improves consistency.
113-119: Improved tooltip implementation.Replacing the generic tooltip with the specialized
RatelimitOverviewTooltipcomponent improves UI consistency. The minor font size adjustment fromtext-smtotext-[13px]maintains the intended design.apps/dashboard/app/auth/sign-in/email-signin.tsx (4)
2-2: Improved component import using shared UI library.Replacing the local input component with
FormInputfrom@unkey/uipackage aligns with the codebase's refactoring goal of centralizing UI components.
32-33: Improved form spacing with consistent grid layout.Increasing the vertical gaps (
gap-16on form andgap-6on container) improves the visual hierarchy and readability of the form.
34-46: Enhanced input field with proper labeling and accessibility.The FormInput implementation adds important accessibility improvements:
- Proper label association with the input
- Consistent styling with the design system
- Better semantics with the fieldset/label structure (from FormInput implementation)
The dark background styling (
dark !bg-black) maintains visual consistency with the authentication UI.
48-51: Simplified button styling while maintaining functionality.The button styling was streamlined by removing the
relativepositioning while preserving all the interactive states and behavior.apps/dashboard/components/logs/details/request-response-details.tsx (2)
1-1: Consolidated tooltip import from a specialized component.Replacing generic tooltip components with the specialized
RatelimitOverviewTooltipcomponent aligns with the codebase's refactoring to remove local UI implementations in favor of shared, reusable components.
86-97: Improved tooltip implementation with consistent positioning.The specialized tooltip implementation:
- Simplifies the component structure by using
asChildto wrap content directly- Provides explicit positioning control with
side: "bottom", align: "end"- Maintains the same functionality while reducing code complexity
- Creates a more consistent tooltip experience across the application
This change is part of a broader refactoring to remove the generic tooltip components in favor of domain-specific implementations.
apps/dashboard/app/(app)/authorization/roles/[roleId]/tree.tsx (2)
3-3: Consolidated tooltip import from a specialized component.Replacing generic tooltip components with the specialized
RatelimitOverviewTooltipcomponent aligns with the codebase's refactoring to standardize tooltip usage.
91-111: Improved tooltip implementation for permission items.The specialized tooltip implementation:
- Simplifies rendering by using
contentprop to define tooltip content- Applies consistent positioning with
side: "top", align: "start"- Uses
asChildto integrate with the existing element structure- Maintains the same functionality while reducing nested component complexity
This change fits well with the PR objective of cleaning up unused components by moving toward standardized UI patterns.
apps/dashboard/app/auth/sign-up/email-signup.tsx (5)
8-8: Improved component import using shared UI library.Replacing local input components with
FormInputfrom@unkey/uialigns with the refactoring goal of centralizing UI components and removing unused local components.
58-59: Enhanced form layout with improved spacing.Increasing the gaps between form elements (
gap-16for the form andgap-10for the container) improves visual hierarchy and readability.
60-83: Improved first and last name field implementation with proper labels.The name fields now:
- Include proper labels for accessibility
- Have consistent styling with dark backgrounds
- Maintain the same layout in a flex row with appropriate spacing
- Include the same validation attributes
This improves both the visual consistency and accessibility of the form.
84-97: Enhanced email input with proper labeling and email-specific attributes.The email field now:
- Includes a proper label for accessibility
- Specifies
autoComplete="email"for browser autofill support- Has consistent styling with the other form fields
- Properly preserves the email value from URL parameters
These improvements enhance both usability and accessibility.
98-110: Well-implemented conditional rendering in submit button.The button implementation:
- Maintains the same loading state behavior
- Has consistent styling with other authentication buttons
- Properly handles client-side hydration to prevent UI flicker
The client-side rendering approach correctly handles the loading spinner state.
apps/dashboard/app/(app)/settings/root-keys/[keyId]/update-root-key-name.tsx (3)
3-12: Good cleanup of imports for the form refactoring.The imports have been appropriately updated to use
FormInputfrom@unkey/uiandControllerfromreact-hook-form, aligning with the broader pattern of consolidating UI components from the shared library.
31-44: Nice simplification of the form state management.The direct destructuring of
handleSubmit,control, andformStatefromuseFormhook is a cleaner approach than nesting these properties within a form object. This pattern makes the code more readable and maintainable.
62-98: Well-structured form with Controller pattern.The form structure has been successfully refactored to use React Hook Form's
Controllerpattern with theFormInputcomponent from@unkey/ui. This approach:
- Simplifies the form markup
- Encapsulates input, error, and description handling into a single component
- Makes the code more maintainable
The Button's
loadingprop is also properly utilized for the loading state.apps/dashboard/app/new/create-api.tsx (4)
8-12: Good import cleanup for form components.The imports have been appropriately updated to use
FormInputfrom@unkey/uiandControllerfromreact-hook-form, consistent with the project-wide refactoring to consolidate UI components.
26-34: Good form hook refactoring.The direct destructuring of
handleSubmit,control,reset, andformStateproperties fromuseFormmakes the code cleaner and more maintainable.
81-104: Well-implemented form with Controller pattern.The form structure has been successfully refactored to use React Hook Form's
Controllerwith theFormInputcomponent. The description is properly handled with JSX elements inside thedescriptionprop.
105-116: Good button state management.The button is correctly using the
isValidflag from form state to disable submission when the form is invalid, and theloadingprop handles the loading state.apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-prefix.tsx (4)
30-43: Well-structured form hook setup.The destructuring of form properties directly from
useFormis clean and follows modern React patterns. UsingisValid,isSubmitting, andisDirtyfor button state management is a good practice.
64-78: Good use of SettingCard component.Using the
SettingCardcomponent from@unkey/uicreates a consistent UI pattern across the application for settings forms.
79-96: Well-implemented Controller with FormInput.The Controller pattern is implemented correctly with proper handling of the onBlur event, which only calls
field.onBlur()if the value is not empty. This maintains good form validation behavior.Based on the retrieved learnings, it's good that you kept the hidden input for
keyAuthIdwhich works correctly without being registered with React Hook Form.
97-107: Good button state handling.The button's disabled state properly accounts for form validity, submission state, and whether the form is dirty. The loading state is also handled correctly.
apps/dashboard/app/(app)/apis/[apiId]/settings/components/update-api-name.tsx (5)
7-10: Good import cleanup for UI components.The imports have been correctly updated to use components from
@unkey/uiandreact-hook-form, consistent with the project-wide refactoring.
31-45: Well-structured form hook setup.The destructuring of form properties from
useFormis clean and follows modern React patterns. The form configuration with validation options is well-organized.
74-85: Good form structure with hidden inputs.The form structure with SettingCard is clean and maintains the necessary hidden inputs for
apiIdandworkspaceId.
86-103: Well-implemented onChange handler in Controller.The custom onChange handler properly prevents updating the field when the input value is empty, which helps maintain form validation integrity.
104-114: Good button state management.The button's disabled state correctly accounts for form validity, submission state, mutation loading, and form dirtiness. The loading state handles both form submission and mutation loading states.
apps/dashboard/app/new/create-workspace.tsx (2)
9-15: Good job on cleanup and standardizationUsing shared UI components from
@unkey/uiand simplifying imports aligns with the PR's goal of removing unused components.
21-28: Form state management is improvedNice refactoring to destructure the React Hook Form properties for cleaner usage in the component.
apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-bytes.tsx (2)
5-8: Successfully unified UI component importsGood replacement of local UI components with the standardized @unkey/ui library.
29-42: Form state handling is improvedDestructuring form state properties and using a cleaner resolver setup improves readability.
apps/dashboard/app/(app)/apis/[apiId]/settings/components/update-ip-whitelist.tsx (3)
7-11: UI component imports properly updatedGood work consolidating component imports from @unkey/ui.
35-46: Form state management properly refactoredDestructuring form state properties improves code clarity and maintainability.
96-123: Form layout correctly simplifiedForm controls are properly implemented with the Controller pattern. The form submission state control using isDirty is good practice.
apps/dashboard/app/(app)/settings/team/invite.tsx (3)
7-20: Import consolidation is well executedGood replacement of local icon and UI components with standardized imports from @unkey/icons and @unkey/ui libraries.
36-47: Form state successfully consolidatedThe form management is properly unified with a single useForm instance and appropriate defaults.
94-110: Dialog interface improvedGood improvement moving the subtitle text into the DialogContainer component and simplifying the footer layout.
apps/dashboard/app/(app)/apis/[apiId]/settings/components/update-ip-whitelist.tsx
Show resolved
Hide resolved
mcstepp
left a comment
mcstepp
left a comment
There was a problem hiding this comment.
just a few code rabbit suggestions, the big one is about the form validation with trimming whitespace. there's a non-blocking refactor suggestion too.
also not sure what the difference is between @/components and @unkey/ui -- is it just that we don't have everything migrated over to @unkey/ui?
|
lemme fix them up |
Yes. Not everything should be in the @unkey/ui, but some important elements like select, checkbox, etc... they are still being migrated there. We actually moved most of them, but we never had enough time to do a proper cleanup, so this PR achieves that." |
mcstepp
left a comment
mcstepp
left a comment
There was a problem hiding this comment.
👍
needs merge conflict fixes though
Yup |
|
I’ll fix the conflict 🫡 |
2 participants
What does this PR do?
This PR removes unused components and refactors some components to use our own
uipackage components.Type of change
How should this be tested?
Some critical changes
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
Refactor
react-hook-formand unified UI components.Removals
Style
Bug Fixes