-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(lang-section): option to show language name and description on a single line #2246
base: main
Are you sure you want to change the base?
Conversation
…cription in a single line. updated templates accordingly
📝 WalkthroughWalkthroughThis update adds conditional styling to the Languages component across multiple templates. The components now apply the CSS classes Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Switch
participant LanguagesDialog
participant FormState
User->>Switch: Toggle "singleLine" option
Switch->>LanguagesDialog: onCheckedChange(newValue)
LanguagesDialog->>FormState: update("singleLine", newValue)
FormState-->>LanguagesDialog: State Updated
sequenceDiagram
participant Template
participant LanguageItem
Template->>LanguageItem: Evaluate singleLine property
alt singleLine true
LanguageItem->>Template: Render with flex layout classes
else singleLine false
LanguageItem->>Template: Render with default styling
end
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/client/src/pages/builder/sidebars/left/dialogs/languages.tsx (1)
90-107
: Well-implemented form field for single line toggle.The FormField implementation is clear and follows the existing patterns in the form. The label accurately describes the feature's purpose.
Consider simplifying the onChange handler:
onCheckedChange={(checked) => { - field.onChange((field.value = checked)); + field.onChange(checked); }}This is more in line with React Hook Form's typical patterns and avoids the side effect of assigning to field.value within the onChange call.
apps/artboard/src/templates/bronzor.tsx (1)
431-436
: Consider potential CSS class conflict in single-line modeYour implementation correctly uses the
cn
utility to combine classes, but there may be a conflict betweenspace-y-0.5
(vertical spacing) and the single-line mode classes that includespace-x-2
(horizontal spacing). WhensingleLine
is true, you might want to conditionally exclude the vertical spacing class.<div className={cn( - "space-y-0.5", + item.singleLine ? "" : "space-y-0.5", item.singleLine ? "flex flex-row items-center space-x-2" : "", )} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/artboard/src/templates/azurill.tsx
(1 hunks)apps/artboard/src/templates/bronzor.tsx
(1 hunks)apps/artboard/src/templates/chikorita.tsx
(1 hunks)apps/artboard/src/templates/ditto.tsx
(1 hunks)apps/artboard/src/templates/gengar.tsx
(1 hunks)apps/artboard/src/templates/glalie.tsx
(1 hunks)apps/artboard/src/templates/kakuna.tsx
(1 hunks)apps/artboard/src/templates/leafish.tsx
(1 hunks)apps/artboard/src/templates/nosepass.tsx
(1 hunks)apps/artboard/src/templates/onyx.tsx
(1 hunks)apps/artboard/src/templates/pikachu.tsx
(1 hunks)apps/artboard/src/templates/rhyhorn.tsx
(1 hunks)apps/client/src/pages/builder/sidebars/left/dialogs/languages.tsx
(2 hunks)libs/schema/src/sections/language.ts
(2 hunks)
🔇 Additional comments (14)
apps/artboard/src/templates/nosepass.tsx (1)
441-441
: Conditional layout for single-line language display implemented correctly.The implementation adds a conditional CSS class that changes the layout to a horizontal flex container when the
singleLine
flag is true, creating a space-efficient display format for language entries.apps/artboard/src/templates/onyx.tsx (1)
430-435
: Good use of the cn utility for conditional class combining.The implementation properly maintains the base spacing class (
space-y-0.5
) while conditionally adding the flex layout classes whensingleLine
is true. Using thecn
utility function is the right approach for combining multiple class conditions.apps/artboard/src/templates/rhyhorn.tsx (1)
430-435
: Consistent implementation of conditional layout classes.This implementation follows the same pattern as the Onyx template, maintaining consistency in how the feature is implemented across similar templates. The base spacing is preserved with
space-y-0.5
while adding flex layout when needed.apps/artboard/src/templates/pikachu.tsx (1)
464-464
: Simple and clean conditional layout implementation.The implementation correctly adds the necessary flex layout classes when
singleLine
is true, matching the implementation style used in other templates like Nosepass.libs/schema/src/sections/language.ts (2)
10-10
: Good addition of singleLine property to the schema.The new property uses
z.coerce.boolean().default(false)
which appropriately coerces values to boolean type and sets a sensible default.
22-22
: Correctly initialized default property value.The default value is properly added to the defaultLanguage object, maintaining consistency with the schema definition.
apps/artboard/src/templates/gengar.tsx (1)
436-436
: Clean implementation of conditional styling for single line display.The conditional class application using
item.singleLine ? "flex flex-row items-center space-x-2" : ""
is a good approach. When enabled, it creates a horizontal flex layout with appropriate spacing between elements.apps/artboard/src/templates/kakuna.tsx (1)
396-396
: Consistent implementation of single line styling.This implementation matches the pattern used in other templates, maintaining consistency throughout the application.
apps/client/src/pages/builder/sidebars/left/dialogs/languages.tsx (1)
12-12
: Correctly imported Switch component.The Switch component is properly imported from the UI library.
apps/artboard/src/templates/leafish.tsx (1)
391-391
: Clean implementation of the single-line optionThe conditional class assignment is a clean approach to implementing the single-line display option for language items. This will allow users to choose between vertical stacking (default) or horizontal layout with proper spacing.
apps/artboard/src/templates/ditto.tsx (1)
458-458
: Clean implementation of the single-line optionThe conditional class assignment is an elegant solution for implementing the single-line display feature. It provides a consistent user experience across different resume templates.
apps/artboard/src/templates/azurill.tsx (1)
420-422
: Good adaptation for the centered layout templateThis implementation correctly adds the
justify-center
class, which is appropriate for the centered design of this template. The horizontal alignment matches the existing design patterns in the Azurill template.apps/artboard/src/templates/glalie.tsx (1)
446-451
: LGTM - Clean implementation of conditional layout.The addition of conditional styling using the
cn
utility allows for flexible display of language items. Whenitem.singleLine
is true, the language name and description will display horizontally with appropriate spacing, otherwise they'll maintain the original vertical stacking.apps/artboard/src/templates/chikorita.tsx (1)
431-436
: LGTM - Consistent implementation across templates.The change applies the same conditional styling pattern as in other templates, ensuring a uniform user experience. The implementation is clean and maintains backward compatibility while adding the new single-line display option.
taking a first stab at closing #2245
Summary by CodeRabbit