refactor: extract FeaturesTableFilters and UI components from FeaturesPage #6314
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
bot
added
the
front-end
talissoncosta
force-pushed
the
refactor/migrate-organisationsettingspage
branch
from
4fd15de to
c08d4de
Compare
talissoncosta
force-pushed
the
refactor/migrate-critical-components-featurespagejs-1
branch
from
5dfaa4e to
798187f
Compare
Create reusable hook that wraps useUpdateProjectMutation with automatic toast notifications, supporting customizable messages and error callbacks.
Replace duplicated try/catch/toast patterns across 5 components with useUpdateProjectWithToast hook, eliminating ~70 lines of duplicated code.
Replace complex Omit/Partial approach with explicit Pick for better clarity and type safety. This makes it immediately clear which Project fields are updateable via the API.
Add RTK Query optimistic updates to updateProject mutation using onQueryStarted lifecycle hook. This immediately updates the cache when mutations are called and automatically rolls back on errors, providing instant UI feedback without manual rollback logic. Benefits: - Instant UI updates before server responds - Automatic error rollback via patchResult.undo() - Reduces component complexity by eliminating manual error handling 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Remove useEffect and manual rollback logic from 3 components that now benefit from mutation-level optimistic updates: - ProjectInformation: Remove useEffect syncing and onError rollback - AdditionalSettings: Remove useEffect syncing - FeatureNameValidation: Remove useEffect syncing and manual optimistic update Components now use local state only for form editing, with RTK Query handling all cache updates and automatic rollbacks on errors. Benefits: - Simpler components (~22 lines removed across 3 files) - No cursor jumps during typing when other settings update - No manual error handling needed 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Create ChangeRequestsApprovalsSetting component following the same pattern as other settings (PreventFlagDefaultsSetting, CaseSensitivitySetting, FeatureNameValidation). Changes: - New ChangeRequestsApprovalsSetting.tsx component - AdditionalSettings simplified from 76 to 25 lines - All settings now follow consistent pattern (receive project prop) Benefits: - Better consistency across all setting components - Improved encapsulation (component owns its state) - Easier to test in isolation - Cleaner AdditionalSettings parent component 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Remove 9 unnecessary useCallback wrappers from project settings components. These callbacks had dependencies that change frequently (project properties, local state), causing recreation on every render and providing no memoization benefit. Changes across 7 files: - CaseSensitivitySetting: Remove useCallback from handleToggle - PreventFlagDefaultsSetting: Remove useCallback from handleToggle - FeatureNameValidation: Remove useCallback from handleToggle + handleSave - ProjectInformation: Remove useCallback from handleSubmit - ChangeRequestsApprovalsSetting: Remove useCallback from both functions - SDKSettingsTab: Remove useCallback from 2 toggle handlers - DeleteProject: Remove useCallback from handleDelete Benefits: - Simpler, more honest code (~36 lines removed) - Same performance (callbacks already recreated every render) - Removed unused useCallback imports 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Call AppActions.refreshOrganisation() after project updates to ensure the legacy OrganisationStore stays in sync with RTK Query cache. This fixes the navbar not updating when project name changes, which was causing the E2E test (project-test.ts) to fail.
talissoncosta
force-pushed
the
refactor/migrate-critical-components-featurespagejs-1
branch
from
41026cd to
dc1f8f9
Compare
talissoncosta
changed the title
talissoncosta
force-pushed
the
refactor/migrate-critical-components-featurespagejs-1
branch
from
dc1f8f9 to
7482180
Compare
…e dependencies Extract the features table filters into a separate component with explicit prop dependencies, removing direct store access. This prepares the component for future RTK Query migration by making it a pure presentational component. Changes: - Create FeaturesTableFilters component accepting filter state as props - Remove FeatureListStore and AccountStore imports from component - Remove redundant FeatureListStore.isLoading mutations (handled by AppActions) - Pass isLoading and orgId as explicit props from parent - Export FeaturesTableFilters from features index - Fix nested ternary linting issue in getFiltersFromParams The component is now framework-agnostic and fully testable with props-based state. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
talissoncosta
force-pushed
the
refactor/migrate-critical-components-featurespagejs-1
branch
from
7482180 to
6ae3524
Compare
Zaimwa9
left a comment
Zaimwa9
left a comment
There was a problem hiding this comment.
Just a couple of initial comments
| @@ -0,0 +1,173 @@ | |||
| import React, { FC } from 'react' | |||
There was a problem hiding this comment.
Opening the discussion. If we go with the features folder. Why not move this component there? And have the larger ones as their own folders?
Overall, how do you foresee the ideal structure ? What will we have in web/components, web/components/pages.
Of course we don't have to do anything now in this PR but it could be interesting to start projecting and go in the future direction
There was a problem hiding this comment.
Great question! Here’s the structure I’m envisioning moving forward:
/pages/ – Feature-Specific Pages and Local Components
These folders contain the main page component and any components that are only used within that specific feature or page.
Examples:
features/organisation-settings/project-settings/account-settings/
/components/ – Shared, Reusable Components
These are components that are used across multiple pages or features.
Examples:
PageHeaderButtonModal
A component moves here only when it has real reuse.
Migration Path
- Start by keeping components inside their feature-specific folders under
/pages/. - When a component begins to be used across multiple places, extract it into
/components/. - This keeps development fast and co-located early on, and only promotes components to shared when there’s clear benefit.
In my view it brings:
- Clear separation between page-level and shared components
- Keeps feature-related code grouped for readability and maintainability
- Avoids premature abstraction
- Makes the shared layer intentional and clean
- Aligns with our incremental refactoring approach
About /components/pages/
Ideally, I’d like to move everything currently under /components/pages/ into /pages/, but I don’t want to introduce too many changes at once. For now, I’m keeping the refactor focused on extracting shared components within the existing structure.
Note on the Migration Start
Also, sorry for not discussing this before starting the migration. In my mind, we were already heading in this direction — we just weren’t placing things into their specific folders yet.
Does this structure make sense for our future direction? Happy to adjust or discuss alternatives!
| )} | ||
| <FeatureMetricsSection | ||
| environmentApiKey={environment?.api_key} | ||
| forceRefetch={this.state.forceMetricsRefetch} |
There was a problem hiding this comment.
So I remember having added that because some components were relying on the store and updating the feature state or identity wouldn't invalidate the correct tags.
Cf:
updateIdentity: builder.mutation<Res['identity'], Req['updateIdentity']>({
invalidatesTags: (res) => [
{ id: 'LIST', type: 'Identity' },
{ id: res?.id, type: 'Identity' },
{ id: 'METRICS', type: 'Environment' },
],
I really don't think the whole FeaturesPage refactoring will be enough but putting it on your radar if ever there is the possibility to get rid of it
| <TableGroupsFilter | ||
| className='me-4' | ||
| projectId={projectId} | ||
| orgId={orgId?.toString()} |
There was a problem hiding this comment.
Sorry to be sooo annoying with this orgIds ahah, reminder to check if we can use number there
There was a problem hiding this comment.
It is all good, I think we should move forward with this one. Does it sounds good to you ?
There was a problem hiding this comment.
I'm up, let me know when the one you pinned is ready
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature!Changes
Refactored FeaturesPage.js by extracting three focused components to improve code organization and maintainability:
New Components Created
FeaturesTableFilters.tsx- Pure TypeScript component managing all filter UI:FeaturesEmptyState.tsx- Empty state component with onboarding information for new usersFeatureMetricsSection.tsx- Conditional metrics display wrapper (feature-flagged)features/index.ts- Barrel export file for the features moduleChanges to Existing Files
FeaturesPage.js- Refactored to use extracted components, reducing file size by ~140 lines (21% reduction)Technical Details
Migration Roadmap
This PR completes Phase 1 of the FeaturesPage migration. Future phases will be handled in separate PRs.
Phase 1: Extract Components ✅ (This PR)
FeaturesTableFilters.tsx(163 lines) - All filter UIFeaturesEmptyState.tsx(109 lines) - Empty state with onboardingFeatureMetricsSection.tsx(32 lines) - Metrics display wrapperfeatures/index.ts- Barrel exportsFuture Phases (Separate PRs)
Phase 2: Create RTK Query Services (2-3 days)
Phase 3: Convert to Functional Component (3-4 days)
useFeatureFilters,useFeatureActionsPhase 4: Deprecate FeatureListProvider (1-2 days)
Phase 5: Testing & Cleanup (2-3 days)
Progress Tracking
How did you test this code?
Manual Testing
Automated Testing
Browser Testing