refactor: GraphQL page redesign#388
Conversation
🦋 Changeset detectedLatest commit: 016add6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis pull request introduces several UI and component improvements across the project, focusing on the Models/GraphQL page. The changes include updating color variables in CSS, introducing a new Changes
Sequence DiagramsequenceDiagram
participant User
participant MultiSwitchFieldRoot
participant MultiSwitchFieldController
participant Form
User->>MultiSwitchFieldRoot: Interact with switches
MultiSwitchFieldRoot->>MultiSwitchFieldController: Update selected options
MultiSwitchFieldController->>Form: Sync form state
Form-->>User: Reflect changes
This sequence diagram illustrates the interaction flow of the new 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Note 🎁 Summarized by CodeRabbit FreeYour organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
kingston
left a comment
kingston
left a comment
There was a problem hiding this comment.
Overall LGTM - but I guess we need to merge the other one. Can you attach a screenshot of it as well?
| --foreground: 229 84% 5%; | ||
| --muted: 210 40% 96%; | ||
| --muted-foreground: 215 16% 47%; | ||
| --muted: 220 14% 96%; |
There was a problem hiding this comment.
to match the colors in this figma design: https://www.figma.com/design/KB9n2YKXzfdWAVVBBOaNJR/Baseplate---Design?node-id=2625-19634&node-type=frame&t=A9d7M5WQHwPuoobB-0
|
@matei-s can you attach a screenshot? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
packages/ui-components/src/components/MultiSwitchField/MultiSwitchField.tsx (2)
19-26: Add JSDoc documentation for the interface.Consider adding JSDoc documentation to describe the purpose and usage of each prop in the
MultiSwitchFieldPropsinterface. This would improve developer experience and maintainability.Example:
+/** + * Props for the MultiSwitchField component + * @template OptionType - The type of the option object + */ export interface MultiSwitchFieldProps<OptionType> extends MultiSelectOptionProps<OptionType>, FieldProps { + /** Currently selected values */ value?: string[]; + /** Callback fired when the selection changes */ onChange?: (value: string[]) => void; + /** Additional CSS classes */ className?: string; + /** Whether the field is disabled */ disabled?: boolean; }
133-134: Consider using type assertion with type predicate.The type casting could be made more type-safe using a type predicate function.
- const restProps = rest as MultiSwitchFieldProps<OptionType> & - AddOptionRequiredFields<OptionType>; + function isMultiSwitchFieldProps( + props: unknown + ): props is MultiSwitchFieldProps<OptionType> & AddOptionRequiredFields<OptionType> { + return true; // Add proper type checking here + } + const restProps = isMultiSwitchFieldProps(rest) ? rest : {};packages/project-builder-web/src/pages/data/models/_constants.tsx (1)
13-13: Fix typo in description stringThe description contains a typo: "value 'password'" should probably be "field 'password'".
- description: "Hashes the input value of value 'password' using Argon2id", + description: "Hashes the input value of field 'password' using Argon2id",packages/project-builder-web/src/pages/data/models/edit.id/_components/graphql/GraphQLObjectTypeSection.tsx (4)
117-144: Fix inconsistent button titlesThe expand/collapse button titles use "fields" for all sections. This should be specific to each section type.
- title={ - shouldCollapseFields - ? 'Expand fields' - : 'Collapse fields' - } + title={ + shouldCollapseFields + ? 'Expand field list' + : 'Collapse field list' + }
190-220: Fix inconsistent button titles for local relationsThe expand/collapse button titles incorrectly use "fields" instead of "relations".
- title={ - shouldCollapseLocalRelations - ? 'Expand fields' - : 'Collapse fields' - } + title={ + shouldCollapseLocalRelations + ? 'Expand local relations' + : 'Collapse local relations' + }
Line range hint
258-283: Remove commented out codeRemove the commented out code block as it's no longer needed and adds unnecessary noise to the codebase.
284-314: Fix inconsistent button titles for foreign relationsThe expand/collapse button titles incorrectly use "fields" instead of "relations".
- title={ - shouldCollapseForeignRelations - ? 'Expand fields' - : 'Collapse fields' - } + title={ + shouldCollapseForeignRelations + ? 'Expand foreign relations' + : 'Collapse foreign relations' + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (48)
.changeset/little-zebras-boil.md(1 hunks).changeset/violet-ads-float.md(1 hunks)packages/project-builder-web/src/index.css(1 hunks)packages/project-builder-web/src/index.tsx(1 hunks)packages/project-builder-web/src/pages/_routes.tsx(1 hunks)packages/project-builder-web/src/pages/data/_layout.tsx(1 hunks)packages/project-builder-web/src/pages/data/_routes.tsx(1 hunks)packages/project-builder-web/src/pages/data/enums/EnumsSidebarList.tsx(1 hunks)packages/project-builder-web/src/pages/data/enums/hooks/useEnumForm.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/_components/ModelsSidebarList.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/_components/NewModelDialog.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/_constants.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/_hooks/useModelForm.ts(1 hunks)packages/project-builder-web/src/pages/data/models/_routes.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/constants/built-in-transformers.ts(0 hunks)packages/project-builder-web/src/pages/data/models/edit.id/_components/BadgeWithTypeLabel.tsx(2 hunks)packages/project-builder-web/src/pages/data/models/edit.id/_components/ModelInfoEditDialog.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/edit.id/_components/ModelRelationsSection.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/edit.id/_components/ModelUniqueConstraintsSection.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/edit.id/_components/fields/ModelAddFieldButton.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/edit.id/_components/fields/ModelFieldDefaultValueInput.tsx(7 hunks)packages/project-builder-web/src/pages/data/models/edit.id/_components/fields/ModelFieldForm.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/edit.id/_components/fields/ModelFieldTypeInput.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/edit.id/_components/fields/badges/ModelFieldBadges.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/edit.id/_components/fields/primary-key/ModelPrimaryKeyForm.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/edit.id/_components/fields/relations/ModelRelationForm.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/edit.id/_components/fields/unique-constraints/ModelUniqueConstraintBadge.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/edit.id/_components/fields/unique-constraints/ModelUniqueConstraintForm.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/edit.id/_components/graphql/GraphQLMutationsSection.tsx(6 hunks)packages/project-builder-web/src/pages/data/models/edit.id/_components/graphql/GraphQLObjectTypeSection.tsx(6 hunks)packages/project-builder-web/src/pages/data/models/edit.id/_components/graphql/GraphQLQueriesSection.tsx(5 hunks)packages/project-builder-web/src/pages/data/models/edit.id/_components/service/ServiceEmbeddedRelationForm.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/edit.id/_components/service/ServiceMethodFieldsSection.tsx(3 hunks)packages/project-builder-web/src/pages/data/models/edit.id/_components/service/ServiceTransformerForm.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/edit.id/_components/service/ServiceTransformersSection.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/edit.id/_layout.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/edit.id/_routes.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/edit.id/graphql.page.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/edit.id/index.page.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/edit.id/service.page.tsx(1 hunks)packages/project-builder-web/src/pages/data/models/edit/_routes.tsx(0 hunks)packages/project-builder-web/src/pages/data/models/edit/schema/schema.page.tsx(0 hunks)packages/project-builder-web/src/pages/data/models/index.page.tsx(2 hunks)packages/ui-components/src/components/InputField/InputField.tsx(3 hunks)packages/ui-components/src/components/MultiSwitchField/MultiSwitchField.tsx(1 hunks)packages/ui-components/src/components/SwitchField/SwitchField.tsx(2 hunks)packages/ui-components/src/components/index.ts(1 hunks)packages/ui-components/src/utils/cn.ts(1 hunks)
💤 Files with no reviewable changes (3)
- packages/project-builder-web/src/pages/data/models/edit/schema/schema.page.tsx
- packages/project-builder-web/src/pages/data/models/edit/_routes.tsx
- packages/project-builder-web/src/pages/data/models/constants/built-in-transformers.ts
✅ Files skipped from review due to trivial changes (3)
- packages/project-builder-web/src/pages/data/enums/EnumsSidebarList.tsx
- packages/project-builder-web/src/pages/data/models/_hooks/useModelForm.ts
- packages/project-builder-web/src/pages/data/models/edit.id/_components/ModelInfoEditDialog.tsx
🚧 Files skipped from review as they are similar to previous changes (38)
- .changeset/little-zebras-boil.md
- packages/project-builder-web/src/pages/data/models/edit.id/_components/ModelUniqueConstraintsSection.tsx
- packages/project-builder-web/src/pages/data/models/edit.id/_components/service/ServiceEmbeddedRelationForm.tsx
- packages/project-builder-web/src/pages/data/models/edit.id/_components/fields/ModelAddFieldButton.tsx
- packages/project-builder-web/src/pages/data/models/edit.id/_components/fields/unique-constraints/ModelUniqueConstraintBadge.tsx
- packages/project-builder-web/src/pages/data/models/edit.id/_components/service/ServiceTransformerForm.tsx
- packages/project-builder-web/src/pages/data/models/edit.id/_components/BadgeWithTypeLabel.tsx
- packages/project-builder-web/src/pages/data/models/_components/ModelsSidebarList.tsx
- packages/ui-components/src/components/index.ts
- .changeset/violet-ads-float.md
- packages/project-builder-web/src/pages/data/models/edit.id/_components/fields/badges/ModelFieldBadges.tsx
- packages/project-builder-web/src/pages/_routes.tsx
- packages/project-builder-web/src/pages/data/_layout.tsx
- packages/project-builder-web/src/pages/data/models/edit.id/_components/ModelRelationsSection.tsx
- packages/ui-components/src/utils/cn.ts
- packages/project-builder-web/src/pages/data/models/edit.id/_components/fields/unique-constraints/ModelUniqueConstraintForm.tsx
- packages/project-builder-web/src/pages/data/models/edit.id/_components/fields/ModelFieldTypeInput.tsx
- packages/project-builder-web/src/index.tsx
- packages/project-builder-web/src/pages/data/enums/hooks/useEnumForm.tsx
- packages/project-builder-web/src/pages/data/models/edit.id/_components/fields/ModelFieldDefaultValueInput.tsx
- packages/project-builder-web/src/pages/data/models/_routes.tsx
- packages/project-builder-web/src/pages/data/models/edit.id/_components/fields/ModelFieldForm.tsx
- packages/project-builder-web/src/pages/data/models/edit.id/_components/fields/primary-key/ModelPrimaryKeyForm.tsx
- packages/project-builder-web/src/pages/data/models/edit.id/_components/fields/relations/ModelRelationForm.tsx
- packages/ui-components/src/components/InputField/InputField.tsx
- packages/project-builder-web/src/pages/data/models/edit.id/_layout.tsx
- packages/project-builder-web/src/pages/data/models/edit.id/_components/graphql/GraphQLQueriesSection.tsx
- packages/project-builder-web/src/pages/data/models/edit.id/_routes.tsx
- packages/project-builder-web/src/pages/data/_routes.tsx
- packages/ui-components/src/components/SwitchField/SwitchField.tsx
- packages/project-builder-web/src/pages/data/models/edit.id/_components/service/ServiceTransformersSection.tsx
- packages/project-builder-web/src/pages/data/models/edit.id/index.page.tsx
- packages/project-builder-web/src/pages/data/models/_components/NewModelDialog.tsx
- packages/project-builder-web/src/pages/data/models/edit.id/graphql.page.tsx
- packages/project-builder-web/src/pages/data/models/index.page.tsx
- packages/project-builder-web/src/pages/data/models/edit.id/_components/service/ServiceMethodFieldsSection.tsx
- packages/project-builder-web/src/pages/data/models/edit.id/service.page.tsx
- packages/project-builder-web/src/pages/data/models/edit.id/_components/graphql/GraphQLMutationsSection.tsx
🔇 Additional comments (9)
packages/ui-components/src/components/MultiSwitchField/MultiSwitchField.tsx (3)
1-18: LGTM! Well-organized imports with proper type definitions.
The imports are properly structured with clear separation between React types, form-related types, and utility functions.
142-144: LGTM! Clean component composition pattern.
The export cleanly combines the root component and controller using Object.assign, following React conventions.
1-144: Consider accessibility and form integration testing.
As this component is part of a larger form system and replaces checkbox functionality with switches:
- Ensure proper ARIA attributes are being passed through to the underlying
SwitchFieldcomponents - Consider adding integration tests with
react-hook-formto verify form state management - Document the differences between this and
MultiCheckboxFieldfor other developers
Let's verify the form integration:
packages/project-builder-web/src/pages/data/models/_constants.tsx (4)
3-6: LGTM! Clean and well-organized imports.
The imports are logically grouped and properly scoped.
8-28: Well-implemented password transformer with proper security considerations!
The implementation correctly:
- Prevents duplicate password transformers
- Verifies the presence of a passwordHash field
- Uses Argon2id for password hashing
25-25: Consider implementing getSummary method
The getSummary method returns an empty array. Consider adding meaningful summary information to help users understand the transformer's configuration.
✅ Verification successful
Based on the search results, I can now provide a final response:
The empty getSummary implementation is intentional for the password transformer
The empty getSummary implementation in the password transformer is by design. This transformer is a built-in configuration that handles password hashing using Argon2id, and its behavior is straightforward enough that it doesn't require additional summary information. The implementation matches other parts of the codebase where transformers provide summaries based on their specific needs - for example, the embedded relation transformer provides detailed summaries while simpler transformers like the password one don't need to.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if other transformers implement getSummary
ast-grep --pattern 'getSummary: () => $$$'
Length of output: 44
Script:
#!/bin/bash
# Let's try a broader search for getSummary implementations
rg "getSummary" -A 3
Length of output: 2280
Script:
#!/bin/bash
# Search for any transformer configurations
rg "transformer" -A 5 packages/project-builder-web/src/pages/data/models/
Length of output: 43232
Script:
#!/bin/bash
# Check the content of the file for more context
cat packages/project-builder-web/src/pages/data/models/_constants.tsx
Length of output: 2239
18-24: Verify password field validation
The allowNewTransformer checks for a 'passwordHash' field but should we also verify that a 'password' field exists since that's what will be transformed?
packages/project-builder-web/src/pages/data/models/edit.id/_components/graphql/GraphQLObjectTypeSection.tsx (2)
7-14: LGTM: Clean import restructuring
The changes align well with the UI redesign objectives, moving towards standardized UI components and consistent icon usage.
Also applies to: 17-21
96-105: Verify sticky header behavior with nested sticky elements
The section uses nested sticky positioning (top-2 on header, top-0 on table headers). Please verify that this works as intended across different scroll scenarios.
✅ Verification successful
Based on the search results, I can now provide a conclusive response about the sticky positioning behavior:
Sticky header implementation is correct and follows a consistent pattern
The sticky positioning is implemented correctly with appropriate z-index layering:
- Section headers use
sticky top-2withbg-backgroundfor proper background coverage - Table headers use
sticky top-0withz-10andbg-background - Field list headers use
sticky -top-0withz-10andbg-white
The different top values create a natural layering that prevents overlap, and each sticky element has the appropriate background color to maintain visual consistency.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other sticky positioned elements that might conflict
rg -n 'sticky' --type tsx
Length of output: 53
Script:
#!/bin/bash
# Search for sticky positioned elements using ripgrep with broader file patterns
rg -n 'sticky' --type ts --type tsx --type jsx --type js
Length of output: 84
Script:
#!/bin/bash
# Find all TypeScript/React files that might contain sticky positioning
fd -e tsx -e ts -e jsx -e js -x grep -l "sticky"
Length of output: 861
Script:
#!/bin/bash
# Search for table headers with sticky positioning
rg -n 'top-0|sticky' --type ts --type tsx --type jsx --type js -A 2 -B 2
Length of output: 102
Script:
#!/bin/bash
# Let's examine the content of these files to understand the sticky positioning usage
for file in $(fd -e tsx -e ts -e jsx -e js -x echo {}) ; do
echo "=== $file ==="
grep -A 5 -B 5 'sticky\|top-[0-9]' "$file" 2>/dev/null
done
Length of output: 131070
Script:
#!/bin/bash
# Specifically check the related model edit components for sticky positioning patterns
cat "./packages/project-builder-web/src/pages/data/models/edit.id/_components/fields/ModelFieldsForm.tsx"
Length of output: 2349
| --muted: 220 14% 96%; | ||
| --muted-foreground: 220 9% 46%; |
There was a problem hiding this comment.
💡 Codebase verification
Update both light and dark theme color tokens for consistency
The verification reveals inconsistencies that need to be addressed:
- The light theme muted colors in
project-builder-webhave been updated to use hue 220, but the dark theme still uses different hues (222 for muted, 215 for muted-foreground) - The base UI component library (
ui-components/src/styles.css) is still using the old color values (210/215 hues)
To maintain design consistency:
- Update dark theme in
project-builder-webto use hue 220 - Sync changes with the UI component library to ensure consistent theming across the application
🔗 Analysis chain
Consider documenting color token changes
The color adjustments align with the Figma design specs, making the muted colors more subtle by reducing saturation while maintaining a consistent hue of 220. However, the corresponding dark theme values (--muted and --muted-foreground) haven't been updated to match the same hue shift.
Let's verify the color token usage across the codebase:
Consider:
- Updating dark theme tokens to maintain hue consistency (220)
- Adding comments to document the color token changes and their purpose
- Creating a color token documentation file to track these design decisions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any hardcoded color values that might need updating
# Look for HSL/RGB values similar to the old muted colors
rg -g '*.{css,scss,tsx,jsx}' '(210 40% 96%|215 16% 47%)'
Length of output: 773
Script:
#!/bin/bash
# Let's check the dark theme values in both files to understand the color scheme better
rg -A 50 -B 5 '@media.*dark' packages/project-builder-web/src/index.css packages/ui-components/src/styles.css
Length of output: 111
Script:
#!/bin/bash
# Let's try a different approach to find dark theme values
# First, let's see the complete content of both CSS files
cat packages/project-builder-web/src/index.css packages/ui-components/src/styles.css
Length of output: 4448
| onChange={(isChecked) => { | ||
| if (isChecked) { | ||
| onChange?.( | ||
| options | ||
| .map(getOptionValue) | ||
| .filter( | ||
| (val) => | ||
| (val === optionValue || value?.includes(val)) ?? | ||
| false, | ||
| ), | ||
| ); | ||
| } else { | ||
| onChange?.(value?.filter((val) => val !== optionValue) ?? []); | ||
| } | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify the onChange handler logic.
The current implementation is somewhat complex and could be made more readable and efficient.
Consider this simplified version:
- onChange={(isChecked) => {
- if (isChecked) {
- onChange?.(
- options
- .map(getOptionValue)
- .filter(
- (val) =>
- (val === optionValue || value?.includes(val)) ??
- false,
- ),
- );
- } else {
- onChange?.(value?.filter((val) => val !== optionValue) ?? []);
- }
- }}
+ onChange={(isChecked) => {
+ const currentValues = value ?? [];
+ const newValues = isChecked
+ ? [...currentValues, optionValue]
+ : currentValues.filter((val) => val !== optionValue);
+ onChange?.(newValues);
+ }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onChange={(isChecked) => { | |
| if (isChecked) { | |
| onChange?.( | |
| options | |
| .map(getOptionValue) | |
| .filter( | |
| (val) => | |
| (val === optionValue || value?.includes(val)) ?? | |
| false, | |
| ), | |
| ); | |
| } else { | |
| onChange?.(value?.filter((val) => val !== optionValue) ?? []); | |
| } | |
| }} | |
| onChange={(isChecked) => { | |
| const currentValues = value ?? []; | |
| const newValues = isChecked | |
| ? [...currentValues, optionValue] | |
| : currentValues.filter((val) => val !== optionValue); | |
| onChange?.(newValues); | |
| }} |
done! |
2 participants
DEPENDS ON #383
implemented the design for the models/graphql page
Notes
MultiSwitchFieldcomponent (pretty much aMultiCheckboxFieldwith switches)Summary by CodeRabbit
Release Notes
UI Improvements
Component Updates
MultiSwitchFieldcomponent for more flexible role and option selection.MultiCheckboxFieldwithMultiSwitchFieldin GraphQL configuration pages.BadgeandLabelcomponents to improve visual presentation.Minor Enhancements