-
Notifications
You must be signed in to change notification settings - Fork 12
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: add journey elements switch #3533
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects.
|
The latest updates on your projects.
|
The latest updates on your projects.
|
92f2473
to
ec83796
Compare
...admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/Accordion/Accordion.tsx
Outdated
Show resolved
Hide resolved
...rc/components/Editor/Slider/Settings/CanvasDetails/Footer/DisplayTitle/DisplayTitle.spec.tsx
Outdated
Show resolved
Hide resolved
d354b6d
to
38638af
Compare
38638af
to
0d5cdd3
Compare
aca2203
to
147c33f
Compare
WalkthroughThis pull request adds six new configuration properties—showHosts, showChatButtons, showReactionButtons, showLogo, showMenu, and showDisplayTitle—to the journey object across tests, stories, and components. Updates include enhanced state toggling via a new useToggleJourneyProperty hook, refactored event handling (for example, in Accordion, Chat, DisplayTitle, Host, Menu, Logo, and Reactions), and adjustments in conditional rendering. Many tests and stories have been updated to use the screen API for consistency. Minor SVG attribute naming adjustments were also made. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant A as Accordion Component
participant H as useToggleJourneyProperty Hook
participant C as Command Handler (useCommand)
participant M as useJourneyUpdateMutation
U->>A: Toggle switch event
A->>H: Call toggleProperty(checked)
H->>C: Create toggle command
C->>M: Execute journey update mutation (with optimistic response)
M-->>C: Return mutation response
C-->>H: Update journey property state
H-->>A: Return updated state
A->>U: Render UI with new configuration
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Chat/Chat.spec.tsxOops! Something went wrong! :( ESLint: 8.57.1 TypeError: Key "rules": Key "react/jsx-indent": Could not find plugin "react". apps/journeys-admin/src/components/Editor/Slider/Content/Goals/Goals.spec.tsxOops! Something went wrong! :( ESLint: 8.57.1 TypeError: Key "rules": Key "react/jsx-indent": Could not find plugin "react". apps/journeys-admin/src/components/Editor/Editor.stories.tsxOops! Something went wrong! :( ESLint: 8.57.1 TypeError: Key "rules": Key "react/jsx-indent": Could not find plugin "react".
✨ Finishing Touches
🪧 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: 1
🔭 Outside diff range comments (1)
libs/journeys/ui/src/components/StepHeader/Logo/Logo.spec.tsx (1)
16-101
: Enhance test coverage for theshowLogo
property.The test suite should be expanded to comprehensively cover the
showLogo
property. Consider adding the following test cases:
- Component behavior when
showLogo
is false- Transitions between
showLogo
states- Edge cases with different combinations of
showLogo
andlogoImageBlock
This will ensure robust testing of the new feature.
🧹 Nitpick comments (18)
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Reactions/ReactionOption/ReactionOption.tsx (1)
34-34
: Consider making spacing values configurable.The hardcoded padding and margin values in the sx prop could be made configurable or use theme spacing for better maintainability and consistency.
- sx={{ p: 1, mr: 1 }} + sx={(theme) => ({ + p: theme.spacing(1), + mr: theme.spacing(1) + })}apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/JourneyAppearance.spec.tsx (2)
61-63
: Consider adding text content verification.While checking the number of
AccordionSummary
components is good for structural testing, consider adding assertions for the expected text content of each summary to ensure UI consistency.await waitFor(() => expect(screen.getAllByTestId('AccordionSummary')).toHaveLength(4) ) +const summaries = screen.getAllByTestId('AccordionSummary') +expect(within(summaries[0]).getByText('Logo')).toBeInTheDocument() +expect(within(summaries[1]).getByText('Menu')).toBeInTheDocument() +expect(within(summaries[2]).getByText('Chat Widget')).toBeInTheDocument() +expect(within(summaries[3]).getByText('Display Title')).toBeInTheDocument()
89-95
: Verify text content for all AccordionSummary components.The test only verifies the text content of the first summary. For comprehensive UI testing, verify the text content of all summaries.
await waitFor(() => expect(screen.getAllByTestId('AccordionSummary')).toHaveLength(4) ) -expect( - within(screen.getAllByTestId('AccordionSummary')[0]).getByText('Logo') -).toBeInTheDocument() +const summaries = screen.getAllByTestId('AccordionSummary') +const expectedTexts = ['Logo', 'Menu', 'Chat Widget', 'Display Title'] +expectedTexts.forEach((text, index) => { + expect(within(summaries[index]).getByText(text)).toBeInTheDocument() +})libs/journeys/ui/src/components/StepHeader/StepHeaderMenu/StepHeaderMenu.tsx (2)
42-43
: Consider renaming variables for better clarity.The logic is correct, but the variable names could be more descriptive:
isEmpty
could beisMenuHidden
since it checks if the menu should be hiddenshowFallback
could beshowAdminFallback
to better indicate its purpose- const isEmpty = journey?.showMenu !== true || menuButtonIcon == null - const showFallback = variant === 'admin' && isEmpty + const isMenuHidden = journey?.showMenu !== true || menuButtonIcon == null + const showAdminFallback = variant === 'admin' && isMenuHidden
61-74
: Consider extracting the IIFE into a separate function for better maintainability.While the IIFE implementation is correct, extracting it into a named function would improve readability and maintainability.
+ const renderIcon = (): ReactElement | null => { + if (isEmpty) + return variant === 'admin' ? ( + <Menu1 sx={{ color: ({ palette }) => palette.grey[700] }} /> + ) : null + + const Icon = isMenu ? X2 : getMenuIcon(menuButtonIcon) + + return ( + <IconButton disabled={variant === 'admin'} onClick={handleClick}> + <Icon sx={{ color: 'white' }} /> + </IconButton> + ) + } + return ( <Box sx={{...}} data-testid="StepHeaderMenu" > - {(() => { - if (isEmpty) - return variant === 'admin' ? ( - <Menu1 sx={{ color: ({ palette }) => palette.grey[700] }} /> - ) : null - - const Icon = isMenu ? X2 : getMenuIcon(menuButtonIcon) - - return ( - <IconButton disabled={variant === 'admin'} onClick={handleClick}> - <Icon sx={{ color: 'white' }} /> - </IconButton> - ) - })()} + {renderIcon()} </Box> )libs/journeys/ui/src/components/StepFooter/HostAvatars/HostAvatars.tsx (3)
10-29
: Consider renamingPlaceholder
to be more specific.The component name could be more descriptive of its specific purpose, such as
HostAvatarPlaceholder
orEmptyHostAvatar
, to better indicate its role in the host avatars context.
51-78
: Consider simplifying the render function's early return condition.The size check in the early return could be combined with the isEmpty check in a more readable way.
- function render(): ReactElement | null { - if (isEmpty && size === 'large') return null - - return journey?.showHosts !== true && showFallback ? ( + function render(): ReactElement | null { + if (isEmpty) { + if (size === 'large') return null + if (!hasPlaceholder) return null + } + + return journey?.showHosts !== true && showFallback ? (
94-134
: Consider extracting common Avatar styles to reduce duplication.The Avatar styles for both small and large placeholders share common properties. Consider extracting these to a common style object or utility function.
+const getPlaceholderStyles = (size: 'small' | 'large') => ({ + color: 'secondary.light', + opacity: 0.5, + backgroundColor: 'transparent', + '&.MuiAvatar-root': { + border: (theme) => + `2px ${size === 'small' ? 'dashed' : 'solid'} ${theme.palette.grey[700]}`, + height: size === 'small' ? '27px' : '46px', + width: size === 'small' ? '27px' : '46px' + } +}) // Then use it in both places: -sx={{ - color: 'secondary.light', - opacity: 0.5, - backgroundColor: 'transparent', - '&.MuiAvatar-root': { - border: (theme) => `2px dashed ${theme.palette.grey[700]}`, - height: '27px', - width: '27px' - } -}} +sx={getPlaceholderStyles('small')} // And: -sx={{ - color: 'secondary.light', - opacity: 0.5, - backgroundColor: 'transparent', - '&.MuiAvatar-root': { - border: (theme) => `2px solid ${theme.palette.grey[700]}`, - height: '46px', - width: '46px' - } -}} +sx={getPlaceholderStyles('large')}libs/journeys/ui/src/components/StepHeader/StepHeader.tsx (1)
76-90
: LGTM! Consider using loose equality for flexibility.The implementation correctly handles the conditional rendering of the title and includes proper text overflow handling. However, consider using loose equality
== true
if you want the title to display whenshowDisplayTitle
is truthy.- {journey?.showDisplayTitle === true && ( + {journey?.showDisplayTitle == true && (libs/journeys/ui/src/components/StepHeader/Logo/Logo.tsx (1)
60-60
: Consider renaminghideLogo
toisLogoHidden
.The variable name could be more explicit about its boolean nature.
- const hideLogo = journey?.showLogo !== true + const isLogoHidden = journey?.showLogo !== trueapps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/DisplayTitle/DisplayTitle.tsx (1)
51-59
: Consider adding type safety to the event handler.While the implementation is correct, the event handler could benefit from explicit typing.
- switchProps={{ - handleToggle: (e) => toggleProperty(e.target.checked), - checked - }} + switchProps={{ + handleToggle: (e: React.ChangeEvent<HTMLInputElement>) => toggleProperty(e.target.checked), + checked + }}libs/journeys/ui/src/components/StepFooter/FooterButtonList/FooterButtonList.spec.tsx (1)
15-33
: Consider adding more edge cases to the test suite.While the current test coverage is good, consider adding tests for:
- Mixed combinations of undefined/null values
- Transition between different visibility states
The test descriptions are clear and the implementation follows best practices.
Also applies to: 35-50, 52-70
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/Accordion/Accordion.tsx (1)
110-110
: Make the aria-label more descriptive.The current aria-label "controlled" is too generic. Consider using a more descriptive label that includes the accordion name.
- inputProps={{ 'aria-label': 'controlled' }} + inputProps={{ 'aria-label': `Toggle ${name}` }}apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/Accordion/Accordion.spec.tsx (1)
110-133
: Enhance test descriptions for clarity.While the test implementation is good, the descriptions could be more specific:
- it('should render switch', () => { + it('should render switch when switchProps are provided', () => {apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/libs/useToggleJourneyProperty/useToggleJourneyProperty.spec.tsx (1)
64-86
: Consider adding error handling tests.While the update functionality is well tested, consider adding tests for:
- Invalid property names
- Error responses from the mutation
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/DisplayTitle/DisplayTitle.spec.tsx (2)
40-43
: Consider using role-based queries for better accessibility testing.While using testId works, role-based queries help ensure accessibility and follow testing best practices. Consider keeping the original role-based approach:
-const accordion = screen.getByTestId('AccordionSummary') +const accordion = screen.getByRole('button', { name: 'Display Title' })The additional assertions for text and checkbox presence are good additions.
71-78
: Maintain consistency in test implementation.The same testId-based selection is repeated across multiple test cases. If keeping the testId approach, consider extracting the selector to a constant for better maintainability.
Also applies to: 96-104, 130-138
libs/journeys/ui/src/components/TemplateView/data.ts (1)
111-117
: LGTM! Consider adding TypeScript types for the new properties.The new visibility control properties are well-structured and align with the PR objectives. However, consider adding explicit TypeScript types for better type safety.
export interface Journey { // ... existing properties + showHosts: boolean | null + showChatButtons: boolean | null + showReactionButtons: boolean | null + showLogo: boolean | null + showMenu: boolean | null + showDisplayTitle: boolean | null }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (12)
apps/journeys-admin/__generated__/GetAdminJourney.ts
is excluded by!**/__generated__/**
apps/journeys-admin/__generated__/GetAdminJourneyWithPlausibleToken.ts
is excluded by!**/__generated__/**
apps/journeys-admin/__generated__/GetJourney.ts
is excluded by!**/__generated__/**
apps/journeys-admin/__generated__/GetPublisherTemplate.ts
is excluded by!**/__generated__/**
apps/journeys-admin/__generated__/JourneyFields.ts
is excluded by!**/__generated__/**
apps/journeys-admin/__generated__/JourneySettingsUpdate.ts
is excluded by!**/__generated__/**
apps/journeys/__generated__/GetJourney.ts
is excluded by!**/__generated__/**
apps/journeys/__generated__/JourneyFields.ts
is excluded by!**/__generated__/**
apps/watch/__generated__/GetJourney.ts
is excluded by!**/__generated__/**
apps/watch/__generated__/JourneyFields.ts
is excluded by!**/__generated__/**
libs/journeys/ui/src/libs/JourneyProvider/__generated__/JourneyFields.ts
is excluded by!**/__generated__/**
libs/journeys/ui/src/libs/useJourneyQuery/__generated__/GetJourney.ts
is excluded by!**/__generated__/**
📒 Files selected for processing (74)
apps/journeys-admin/src/components/Editor/Editor.spec.tsx
(1 hunks)apps/journeys-admin/src/components/Editor/Editor.stories.tsx
(1 hunks)apps/journeys-admin/src/components/Editor/Slider/Content/Goals/Goals.spec.tsx
(1 hunks)apps/journeys-admin/src/components/Editor/Slider/JourneyFlow/nodes/SocialPreviewNode/SocialPreviewNode.spec.tsx
(1 hunks)apps/journeys-admin/src/components/Editor/Slider/JourneyFlow/nodes/SocialPreviewNode/SocialPreviewNode.stories.tsx
(1 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Chat/Chat.spec.tsx
(3 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Chat/Chat.tsx
(2 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/DisplayTitle/DisplayTitle.spec.tsx
(5 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/DisplayTitle/DisplayTitle.tsx
(2 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Host/Host.spec.tsx
(4 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Host/Host.tsx
(3 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Host/HostList/HostListItem/HostListItem.spec.tsx
(7 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Host/HostList/HostListItem/HostListItem.stories.tsx
(1 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/JourneyAppearance.spec.tsx
(3 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Logo/Logo.spec.tsx
(8 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Logo/Logo.tsx
(3 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Menu/Menu.spec.tsx
(2 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Menu/Menu.tsx
(1 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Menu/MenuIconSelect/MenuIconSelect.spec.tsx
(0 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Menu/MenuIconSelect/MenuIconSelect.tsx
(3 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Reactions/ReactionOption/ReactionOption.tsx
(2 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Reactions/Reactions.spec.tsx
(3 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Reactions/Reactions.tsx
(3 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/libs/useToggleJourneyProperty/index.ts
(1 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/libs/useToggleJourneyProperty/useToggleJourneyProperty.spec.tsx
(1 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/libs/useToggleJourneyProperty/useToggleJourneyProperty.ts
(1 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/Accordion/Accordion.spec.tsx
(1 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/Accordion/Accordion.tsx
(5 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Card/BackgroundColor/BackgroundColor.spec.tsx
(1 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Card/BackgroundMedia/BackgroundMedia.stories.tsx
(1 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Card/BackgroundMedia/Image/BackgroundMediaImage.spec.tsx
(1 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Card/CardLayout/CardLayout.spec.tsx
(1 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Card/CardLayout/CardLayout.stories.tsx
(1 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Card/CardStyling/CardStyling.spec.tsx
(1 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Card/CardStyling/CardStyling.stories.tsx
(1 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/controls/Action/Action.stories.tsx
(1 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/Drawer/VideoBlockEditor/Settings/Poster/Library/VideoBlockEditorSettingsPosterLibrary.spec.tsx
(1 hunks)apps/journeys-admin/src/components/Editor/Slider/Settings/SocialDetails/SocialDetails.stories.tsx
(1 hunks)apps/journeys-admin/src/libs/blockDeleteUpdate/blockDeleteUpdate.spec.ts
(1 hunks)apps/journeys-admin/src/libs/useJourneyUpdateMutation/useJourneyUpdateMutation.ts
(1 hunks)apps/journeys/src/components/Conductor/Conductor.spec.tsx
(1 hunks)apps/journeys/src/components/Conductor/Conductor.stories.tsx
(1 hunks)apps/journeys/src/components/EmbeddedPreview/EmbeddedPreview.spec.tsx
(1 hunks)apps/journeys/src/components/EmbeddedPreview/EmbeddedPreview.stories.tsx
(1 hunks)libs/journeys/ui/src/components/Step/Step.spec.tsx
(1 hunks)libs/journeys/ui/src/components/StepFooter/ChatButtons/ChatButtons.spec.tsx
(8 hunks)libs/journeys/ui/src/components/StepFooter/ChatButtons/ChatButtons.tsx
(3 hunks)libs/journeys/ui/src/components/StepFooter/FooterButtonList/FooterButtonList.spec.tsx
(2 hunks)libs/journeys/ui/src/components/StepFooter/FooterButtonList/FooterButtonList.tsx
(1 hunks)libs/journeys/ui/src/components/StepFooter/HostAvatars/HostAvatars.spec.tsx
(2 hunks)libs/journeys/ui/src/components/StepFooter/HostAvatars/HostAvatars.stories.tsx
(1 hunks)libs/journeys/ui/src/components/StepFooter/HostAvatars/HostAvatars.tsx
(3 hunks)libs/journeys/ui/src/components/StepFooter/HostTitleLocation/HostTitleLocation.spec.tsx
(1 hunks)libs/journeys/ui/src/components/StepFooter/StepFooter.spec.tsx
(6 hunks)libs/journeys/ui/src/components/StepFooter/StepFooter.stories.tsx
(1 hunks)libs/journeys/ui/src/components/StepFooter/StepFooter.tsx
(3 hunks)libs/journeys/ui/src/components/StepHeader/InformationButton/InformationButton.spec.tsx
(1 hunks)libs/journeys/ui/src/components/StepHeader/InformationButton/InformationButton.stories.tsx
(1 hunks)libs/journeys/ui/src/components/StepHeader/Logo/Logo.spec.tsx
(3 hunks)libs/journeys/ui/src/components/StepHeader/Logo/Logo.tsx
(2 hunks)libs/journeys/ui/src/components/StepHeader/StepHeader.spec.tsx
(3 hunks)libs/journeys/ui/src/components/StepHeader/StepHeader.stories.tsx
(1 hunks)libs/journeys/ui/src/components/StepHeader/StepHeader.tsx
(1 hunks)libs/journeys/ui/src/components/StepHeader/StepHeaderMenu/StepHeaderMenu.spec.tsx
(8 hunks)libs/journeys/ui/src/components/StepHeader/StepHeaderMenu/StepHeaderMenu.tsx
(2 hunks)libs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.spec.tsx
(1 hunks)libs/journeys/ui/src/components/TemplateView/TemplateFooter/data.ts
(1 hunks)libs/journeys/ui/src/components/TemplateView/data.ts
(1 hunks)libs/journeys/ui/src/libs/JourneyProvider/JourneyProvider.mock.ts
(1 hunks)libs/journeys/ui/src/libs/JourneyProvider/journeyFields.tsx
(1 hunks)libs/journeys/ui/src/libs/rtl/rtl.spec.tsx
(1 hunks)libs/journeys/ui/src/libs/useJourneyQuery/useJourneyQuery.spec.tsx
(1 hunks)libs/shared/ui/src/components/icons/FlipLeft.tsx
(1 hunks)libs/shared/ui/src/components/icons/FlipRight.tsx
(1 hunks)
💤 Files with no reviewable changes (1)
- apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Menu/MenuIconSelect/MenuIconSelect.spec.tsx
✅ Files skipped from review due to trivial changes (4)
- libs/shared/ui/src/components/icons/FlipLeft.tsx
- libs/shared/ui/src/components/icons/FlipRight.tsx
- apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Host/HostList/HostListItem/HostListItem.stories.tsx
- apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/libs/useToggleJourneyProperty/index.ts
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Deploy Preview and Test (watch, 3533/merge, pull_request, 20)
- GitHub Check: Deploy Preview and Test (videos-admin, 3533/merge, pull_request, 20)
- GitHub Check: Deploy Preview and Test (journeys-admin, 3533/merge, pull_request, 20)
- GitHub Check: test (20, 2/3)
- GitHub Check: lint (20)
🔇 Additional comments (98)
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Card/CardLayout/CardLayout.stories.tsx (1)
76-82
: LGTM! The new journey properties are correctly initialized.The addition of these properties aligns with the PR objectives to add switch features for journey elements. All properties are properly initialized to
null
, maintaining consistency with the existing pattern in the journey object.apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Card/CardStyling/CardStyling.stories.tsx (1)
76-82
: LGTM! The new journey properties are correctly initialized.The new properties (
showHosts
,showChatButtons
,showReactionButtons
,showLogo
,showMenu
,showDisplayTitle
) are properly added to the journey object and initialized tonull
, maintaining consistency with other nullable properties in the object.apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Reactions/ReactionOption/ReactionOption.tsx (2)
3-3
: LGTM!The addition of FormControlLabel import aligns with the new implementation and follows Material-UI conventions.
27-38
: LGTM! Clean implementation using FormControlLabel.The change to use FormControlLabel is a good improvement as it:
- Follows Material-UI best practices for form controls
- Maintains proper accessibility by associating the label with the checkbox
- Preserves all necessary functionality
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Menu/MenuIconSelect/MenuIconSelect.tsx (3)
126-132
: LGTM! Well-balanced styling approach.The combination of
autoWidth
,min-content
width, and minimum dimensions creates a responsive yet constrained select component that maintains good usability.
152-153
: LGTM! Proper spacing implementation.The padding and margin values follow Material-UI's spacing system and create appropriate spacing between elements.
163-163
: LGTM! Proper box sizing model.Adding
boxSizing: 'border-box'
ensures the dashed border doesn't affect the Box's final dimensions.libs/journeys/ui/src/components/StepHeader/StepHeaderMenu/StepHeaderMenu.spec.tsx (4)
75-91
: LGTM! Good test coverage for the newshowMenu
property.The new test case effectively verifies that neither the fallback nor the menu icon is rendered when
showMenu
is false, ensuring proper visibility control.
34-162
: LGTM! Comprehensive test coverage for journey scenarios.The test suite thoroughly covers all essential scenarios:
- Menu icon visibility based on icon and showMenu states
- Button state verification
- Navigation behavior
- Close icon functionality
165-216
: LGTM! Well-structured admin test cases.The admin test suite effectively verifies:
- Fallback icon rendering when
showMenu
is falsy- Button disabled state in admin view
- Prevention of navigation
1-217
: LGTM! Well-structured test organization.The test file follows testing best practices with:
- Clear test grouping and descriptive names
- Proper setup of mocks and test data
- Consistent assertion patterns
- Comprehensive coverage of component behavior
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/JourneyAppearance.spec.tsx (1)
2-2
: LGTM!The addition of the
within
utility from testing-library is appropriate for testing nested elements withinAccordionSummary
components.apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Host/HostList/HostListItem/HostListItem.spec.tsx (5)
1-1
: LGTM! Following Testing Library best practices.The addition of
screen
import aligns with Testing Library's recommended approach for querying elements.
21-23
: LGTM! Consistent use of screen queries.The changes follow Testing Library's best practices by using
screen
queries consistently throughout the test case.Also applies to: 27-27
47-47
: LGTM! Proper screen query usage.The changes maintain test integrity while adopting Testing Library's recommended screen query pattern.
Also applies to: 49-49
81-81
: LGTM! Consistent screen query usage with fireEvent.The change properly integrates screen queries with fireEvent while maintaining the test's functionality.
5-85
: Consider adding test cases for new journey properties.The AI summary mentions new journey properties (
showHosts
,showChatButtons
, etc.) being added across components. Consider adding test cases to verify how theHostListItem
component interacts with these new properties, particularlyshowHosts
if it affects the component's behavior.Would you like me to help generate additional test cases for the new journey properties?
libs/journeys/ui/src/components/StepHeader/Logo/Logo.spec.tsx (4)
18-27
: Add test coverage forshowLogo: false
case.The test only covers the case where
showLogo
is true. Consider adding a test case to verify the component's behavior whenshowLogo
is false.
42-47
: Add test coverage forshowLogo: false
case with nulllogoImageBlock
.The test only covers the case where
showLogo
is true andlogoImageBlock
is null. Consider adding a test case to verify the component's behavior whenshowLogo
is false andlogoImageBlock
is null.
59-63
: Maintain consistency by addingshowLogo
property.For consistency with other test cases and to accurately represent the component's behavior in the editor variant, consider adding the
showLogo
property to this test case.
81-90
: Add test coverage for navigation whenshowLogo
is false.The test only covers the navigation behavior when
showLogo
is true. Consider adding a test case to verify that navigation is disabled or the component is not rendered whenshowLogo
is false.libs/journeys/ui/src/components/StepHeader/StepHeaderMenu/StepHeaderMenu.tsx (3)
6-6
: LGTM!The import of the
Menu1
icon is correctly added and follows the project's conventions.
49-49
: LGTM!The styling changes effectively provide visual feedback in admin mode when the menu is hidden.
Also applies to: 57-57
13-77
: LGTM! Well-structured implementation of the journey's show properties feature.The changes effectively implement the menu visibility control while maintaining appropriate behavior for both admin and user modes. The code is well-organized and follows React best practices.
libs/journeys/ui/src/components/StepFooter/HostAvatars/HostAvatars.tsx (2)
31-37
: LGTM!The interface update with the optional
showFallback
property is well-structured and aligns with the journey elements switch feature.
39-45
: LGTM!The parameter updates are consistent with the interface changes, and the default value is appropriate.
libs/journeys/ui/src/components/StepFooter/StepFooter.tsx (4)
74-78
: LGTM! Clear conditional rendering for mobile reaction buttons.The explicit opt-in check
journey?.showReactionButtons === true
aligns with the PR objectives and ensures clear control over the feature visibility.
112-112
: Verify the purpose of adding showFallback.The addition of the
showFallback
prop seems unrelated to the PR objectives of adding journey element switches. Please clarify the purpose of this change.
117-134
: LGTM! Clear conditional rendering for title and host details.The explicit opt-in checks for
showDisplayTitle
andshowHosts
are well-structured and align with the PR objectives.
138-142
: LGTM! Consistent desktop reaction buttons implementation.The implementation maintains consistency with the mobile view while ensuring proper desktop-only display.
libs/journeys/ui/src/components/StepHeader/StepHeader.tsx (1)
76-76
: Verify the default value ofshowDisplayTitle
.The strict equality check
=== true
means the title will only be displayed whenshowDisplayTitle
is explicitlytrue
. Ensure this aligns with the expected default behavior whenshowDisplayTitle
is undefined/null.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check the default value of showDisplayTitle in the codebase. # Search for showDisplayTitle initialization or default value rg -A 5 $'showDisplayTitle' # Search for journey object type definition ast-grep --pattern $'interface Journey { $$$ showDisplayTitle?: $_ $$$ }'Length of output: 70303
Verification of
showDisplayTitle
default valueOur search shows that in the GraphQL type definitions and generated types (e.g. in GetJourney.ts, globalTypes.ts, and JourneyFields.ts), the field is declared as either a boolean or null (i.e. showDisplayTitle?: boolean | null). Many test cases explicitly set showDisplayTitle to null. This means that if no explicit value is provided, the field will be null—and the strict check (journey?.showDisplayTitle === true) will prevent the title from being rendered. This appears to be intentional per the PR's design.
- The GraphQL interfaces do not assign an explicit default value for showDisplayTitle.
- Several tests and generated type declarations indicate that its absence is represented as null.
- The strict equality check ensures that the Typography component is rendered only when showDisplayTitle is explicitly true.
Based on this information, the implementation aligns with the intended behavior. If the goal is to require an explicit true to display the title, then the current check is correct. Otherwise, you might consider handling undefined/null differently.
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Menu/Menu.tsx (1)
8-8
: LGTM! Clean implementation of the toggle functionality.The integration of
useToggleJourneyProperty
hook with the Accordion's switch props is well-implemented. The toggle function correctly updates the menu visibility state.Also applies to: 15-15, 18-25
libs/journeys/ui/src/components/StepFooter/FooterButtonList/FooterButtonList.tsx (1)
10-12
: LGTM! Good optimization and proper typing.The changes improve the component by:
- Adding proper typing for style props
- Optimizing rendering with early return
- Correctly passing sx prop to Stack
Also applies to: 14-16, 23-23, 26-26
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Menu/Menu.spec.tsx (1)
2-2
: LGTM! Test improvements enhance reliability.The test updates improve specificity and coverage by:
- Using
within
for scoped queries- Testing the checkbox role
- Using proper test IDs
Also applies to: 22-25, 27-27
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/libs/useToggleJourneyProperty/useToggleJourneyProperty.ts (2)
7-17
: LGTM! Well-typed and flexible hook implementation.The hook is well-designed with:
- Strong typing for journey fields
- Clear and focused responsibility
- Support for multiple journey properties
22-47
: LGTM! Robust command pattern implementation with optimistic updates.The implementation includes:
- Proper error handling with early return
- Command pattern for undo/redo support
- Optimistic updates for better UX
Let's verify the test coverage for this hook:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for test file and its coverage # Look for test file fd "useToggleJourneyProperty.spec" --type f # Check test coverage if test file exists if [ $? -eq 0 ]; then echo "Test file found. Checking coverage..." rg -A 5 "describe\('useToggleJourneyProperty" else echo "No test file found for useToggleJourneyProperty hook" fiLength of output: 1554
Test Coverage Verified for useToggleJourneyProperty Hook
The test file for the hook exists at
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/libs/useToggleJourneyProperty/useToggleJourneyProperty.spec.tsx
and includes test cases (e.g., verifying that the property returns true when expected). The coverage confirms robust testing for the hook's implementation.apps/journeys-admin/src/libs/useJourneyUpdateMutation/useJourneyUpdateMutation.ts (1)
44-49
: LGTM! New journey properties added consistently.The new fields for controlling visibility of journey elements are well-structured and follow a consistent naming pattern.
libs/journeys/ui/src/libs/JourneyProvider/JourneyProvider.mock.ts (1)
64-69
: LGTM! Mock journey object updated with new properties.The mock journey object correctly reflects the new visibility control properties, with appropriate null initialization.
libs/journeys/ui/src/components/StepHeader/Logo/Logo.tsx (1)
11-46
: LGTM! Well-structured Fallback and Empty components.The new components provide clear visual feedback with consistent styling and appropriate use of opacity and alpha values.
libs/journeys/ui/src/libs/JourneyProvider/journeyFields.tsx (1)
102-107
: LGTM! GraphQL fragment updated with new journey properties.The new visibility control fields are consistently added to the JourneyFields fragment, maintaining alignment with the mutation and mock objects.
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/DisplayTitle/DisplayTitle.tsx (1)
12-12
: LGTM! Clean hook integration.The useToggleJourneyProperty hook is correctly imported and initialized for managing the showDisplayTitle state.
Also applies to: 19-19
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Chat/Chat.spec.tsx (1)
14-32
: LGTM! Well-structured test implementation.The test implementation follows React Testing Library best practices:
- Uses screen queries instead of destructuring render
- Comprehensive coverage of component rendering
- Clear assertions for checkbox states
Also applies to: 62-63
libs/journeys/ui/src/libs/rtl/rtl.spec.tsx (1)
61-66
: LGTM! Consistent type definition updates.The new visibility properties are correctly added to the journey object with consistent null initialization.
libs/journeys/ui/src/components/StepHeader/InformationButton/InformationButton.stories.tsx (1)
67-73
: LGTM! New journey properties added.The new properties for controlling visibility of UI elements are well-structured and consistently named.
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Reactions/Reactions.tsx (4)
8-8
: LGTM! Icon and hook imports.The ThumbsUp icon is more appropriate for reactions than Share icon. The useToggleJourneyProperty hook addition follows best practices.
Also applies to: 12-12
27-29
: LGTM! Hook implementation.The useToggleJourneyProperty hook is correctly implemented for managing the showReactionButtons state.
64-72
: LGTM! Accordion with switch props.The Accordion component is properly configured with switch props for toggling reactions visibility.
73-73
: LGTM! Stack padding.The added left padding improves the visual hierarchy of the reaction options.
libs/journeys/ui/src/components/StepHeader/StepHeader.spec.tsx (2)
46-48
: LGTM! Journey properties in test.The test correctly includes the new visibility control properties with appropriate values.
64-101
: LGTM! Comprehensive test coverage.The new test case thoroughly verifies that elements are hidden when their respective show properties are turned off.
libs/journeys/ui/src/libs/useJourneyQuery/useJourneyQuery.spec.tsx (1)
80-86
: LGTM! Journey properties in query test.The test data structure is correctly updated with the new visibility control properties.
libs/journeys/ui/src/components/StepFooter/HostAvatars/HostAvatars.stories.tsx (1)
75-81
: LGTM! The new journey properties are well-structured.The new properties follow a consistent naming pattern and their default values align with the component's purpose.
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Chat/Chat.tsx (1)
12-12
: LGTM! The toggle functionality is well-implemented.The
useToggleJourneyProperty
hook is properly integrated with theAccordion
component's switch, providing a clean and type-safe way to toggle chat buttons visibility.Also applies to: 19-19, 44-47
libs/journeys/ui/src/components/StepFooter/HostTitleLocation/HostTitleLocation.spec.tsx (1)
66-72
: LGTM! Test data is properly updated.The journey object in test data is correctly updated with the new properties, maintaining consistency with the type changes.
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/Accordion/Accordion.tsx (1)
104-112
: 🏁 Script executed:#!/bin/bash # Description: Find affected test files that use getByRole to query accordions # Search for getByRole usage in test files rg -A 2 "getByRole.*accordion" "**/*.spec.tsx"Length of output: 102
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/controls/Action/Action.stories.tsx (1)
72-78
: LGTM! Journey configuration properties added.The addition of new journey properties (showHosts, showChatButtons, showReactionButtons, showLogo, showMenu, showDisplayTitle) aligns with the PR objectives of enhancing journey element configurability.
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Reactions/Reactions.spec.tsx (1)
38-47
: Great improvements to test maintainability!The updates to use
screen.getByTestId()
,within()
, and more specific assertions follow testing-library best practices and make the tests more maintainable.libs/journeys/ui/src/components/StepFooter/HostAvatars/HostAvatars.spec.tsx (1)
67-73
: LGTM! Journey properties and test improvements added.The changes maintain consistency with the codebase-wide updates to journey properties while improving test maintainability through the use of screen API methods.
apps/journeys/src/components/EmbeddedPreview/EmbeddedPreview.stories.tsx (1)
98-104
: LGTM! Journey properties added consistently.The addition of journey configuration properties maintains consistency with the codebase-wide updates and supports the new switch feature requirements.
apps/journeys/src/components/Conductor/Conductor.stories.tsx (1)
98-104
: LGTM! New journey properties added consistently.The new visibility control properties are added following the existing pattern and properly initialized.
libs/journeys/ui/src/components/StepHeader/StepHeader.stories.tsx (1)
76-82
: LGTM! Consistent implementation of journey properties.The visibility control properties are added consistently with other story files.
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/Accordion/Accordion.spec.tsx (1)
135-160
: LGTM! Toggle event test implementation.The test properly verifies the switch click handler functionality.
libs/journeys/ui/src/components/TemplateView/TemplateFooter/data.ts (1)
159-165
: LGTM! Journey properties added consistently.The visibility control properties are added following the established pattern across the codebase.
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/libs/useToggleJourneyProperty/useToggleJourneyProperty.spec.tsx (2)
21-62
: LGTM! Comprehensive initial state tests.The tests thoroughly cover all possible initial states (true, false, null) for journey properties.
88-136
: LGTM! Well-structured undo/redo tests.The undo/redo functionality is thoroughly tested with proper assertions and mocks.
apps/journeys-admin/src/components/Editor/Editor.stories.tsx (1)
85-91
: LGTM! Consistent addition of new journey properties.The new properties align with the PR objectives and follow the existing pattern of initialization.
apps/journeys-admin/src/components/Editor/Slider/Settings/SocialDetails/SocialDetails.stories.tsx (1)
77-83
: LGTM! Consistent journey property updates.The new properties maintain consistency with other components and follow the established pattern.
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Host/Host.tsx (3)
18-18
: LGTM! Clean hook import.The import of
useToggleJourneyProperty
hook is well-placed and follows the established import pattern.
64-64
: LGTM! Proper hook usage.The
useToggleJourneyProperty
hook is correctly used to manage the visibility state of hosts.
131-134
: LGTM! Well-structured switch props.The switch props are properly configured with:
- Correct event handling for toggle
- Proper state management via checked prop
apps/journeys-admin/src/libs/blockDeleteUpdate/blockDeleteUpdate.spec.ts (1)
126-132
: LGTM! Comprehensive journey property additions.The new journey properties are properly added and consistently initialized to null, aligning with the feature requirements.
apps/journeys-admin/src/components/Editor/Slider/Content/Goals/Goals.spec.tsx (1)
155-161
: LGTM! Consistent journey property additions.The journey properties are added consistently with other files, maintaining a uniform data structure across the application.
apps/journeys-admin/src/components/Editor/Slider/JourneyFlow/nodes/SocialPreviewNode/SocialPreviewNode.stories.tsx (1)
144-150
: LGTM! Consistent journey property additions.The journey properties are added consistently with other files, ensuring uniform data structure across stories and tests.
libs/journeys/ui/src/components/StepHeader/InformationButton/InformationButton.spec.tsx (1)
85-91
: LGTM!The new journey properties are correctly added and initialized to null, aligning with the PR objectives.
libs/journeys/ui/src/components/StepFooter/ChatButtons/ChatButtons.tsx (2)
35-73
: Well-structured helper functions!The separation into Fallback and Empty functions improves code organization and reusability. The styling is consistent, and the functions handle their specific use cases effectively.
177-199
: Clean conditional rendering implementation!The use of an IIFE with clear conditions improves readability and maintainability. The logic for showing different states based on showChatButtons and chatButtons array is well-structured.
apps/journeys-admin/src/components/Editor/Slider/JourneyFlow/nodes/SocialPreviewNode/SocialPreviewNode.spec.tsx (1)
93-99
: LGTM!The new journey properties are correctly added and initialized to null, maintaining consistency with the PR objectives.
apps/journeys/src/components/EmbeddedPreview/EmbeddedPreview.spec.tsx (1)
95-101
: LGTM!The new journey properties are correctly added and initialized to null, consistent with the PR objectives.
apps/journeys-admin/src/components/Editor/Editor.spec.tsx (1)
99-105
: LGTM!The new journey properties are correctly initialized to null and follow the existing pattern.
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Card/BackgroundMedia/BackgroundMedia.stories.tsx (1)
87-93
: LGTM!The new journey properties are correctly initialized to null and follow the existing pattern.
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Host/Host.spec.tsx (1)
2-8
: Great improvement in test reliability!The changes enhance test robustness by:
- Using the
within
utility for more specific element queries- Verifying the presence of elements before interaction
- Using proper test IDs and roles
Also applies to: 153-158
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Logo/Logo.tsx (1)
33-33
: Well-implemented toggle functionality!The changes:
- Properly integrate the useToggleJourneyProperty hook
- Add switch functionality to the Accordion component
- Follow React best practices for state management
- Maintain type safety with proper props
Also applies to: 58-58, 210-218
libs/journeys/ui/src/components/StepFooter/ChatButtons/ChatButtons.spec.tsx (3)
2-2
: LGTM! Import organization is clean and clear.The imports are well-organized, with testing utilities, generated types, and component imports properly separated.
Also applies to: 5-5, 14-14
54-59
: LGTM! Journey object initialization is simplified.Good use of the
defaultJourney
spread operator to reduce boilerplate and improve maintainability.
105-122
: LGTM! Test coverage for visibility control is comprehensive.The test cases thoroughly verify the component's behavior:
- Visibility when
showChatButtons
is true- Placeholder button visibility for admin users
- Empty state visibility for admin users
Also applies to: 168-202, 193-244
libs/journeys/ui/src/components/StepFooter/StepFooter.spec.tsx (3)
74-80
: LGTM! Journey properties are well-defined.The new visibility control properties are properly typed and initialized.
83-88
: LGTM! Toggle options are well-organized.Good use of a separate object to manage visibility flags, improving code maintainability.
172-184
: LGTM! Test coverage for visibility control is thorough.The test cases properly verify:
- Display title visibility
- Reaction buttons visibility
Also applies to: 227-239
libs/journeys/ui/src/components/StepFooter/StepFooter.stories.tsx (1)
79-85
: LGTM! Journey properties in stories match test files.The visibility control properties are consistently defined across stories and tests.
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Card/CardLayout/CardLayout.spec.tsx (1)
73-79
: LGTM! Journey properties are consistent across components.The visibility control properties are consistently defined across different components.
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Card/CardStyling/CardStyling.spec.tsx (1)
86-92
: LGTM!The new journey properties are correctly added and initialized to
null
, maintaining consistency with the journey object structure across the codebase.apps/journeys/src/components/Conductor/Conductor.spec.tsx (1)
155-161
: LGTM!The new journey properties are correctly added and initialized to
null
in thedefaultJourney
object, maintaining consistency with the journey object structure across the codebase.apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Card/BackgroundColor/BackgroundColor.spec.tsx (1)
79-85
: LGTM!The new journey properties are correctly added and initialized to
null
, maintaining consistency with the journey object structure across the codebase.libs/journeys/ui/src/components/Step/Step.spec.tsx (1)
108-114
: LGTM!The new journey properties are correctly added and initialized to
null
, maintaining consistency with the journey object structure across the codebase.apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/JourneyAppearance/Logo/Logo.spec.tsx (1)
183-183
: Test interaction pattern has been updated.The test interaction pattern has been updated from using button role 'Logo' to using testId 'AccordionSummary', which aligns with the PR objectives of enhancing the Accordion component's functionality.
Also applies to: 230-230, 284-284, 342-342, 390-390, 439-439, 479-479, 527-527
libs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.spec.tsx (1)
75-81
: Journey object properties have been consistently updated.The journey object has been enhanced with new properties for controlling UI elements visibility: showHosts, showChatButtons, showReactionButtons, showLogo, showMenu, and showDisplayTitle. This aligns with the PR objectives of adding switch features for footer properties.
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Card/BackgroundMedia/Image/BackgroundMediaImage.spec.tsx (1)
109-115
: Journey object properties have been consistently updated.The journey object has been enhanced with new properties for controlling UI elements visibility: showHosts, showChatButtons, showReactionButtons, showLogo, showMenu, and showDisplayTitle. This aligns with the PR objectives of adding switch features for footer properties.
apps/journeys-admin/src/components/Editor/Slider/Settings/Drawer/VideoBlockEditor/Settings/Poster/Library/VideoBlockEditorSettingsPosterLibrary.spec.tsx (1)
109-115
: Journey object properties have been consistently updated.The journey object has been enhanced with new properties for controlling UI elements visibility: showHosts, showChatButtons, showReactionButtons, showLogo, showMenu, and showDisplayTitle. This aligns with the PR objectives of adding switch features for footer properties.
<Box> | ||
<ChatButtons /> | ||
</Box> |
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.
🛠️ Refactor suggestion
Add showChatButtons condition for consistency.
The ChatButtons component should follow the same pattern as other journey elements and be controlled by the showChatButtons
property.
Apply this diff to maintain consistency:
- <Box>
- <ChatButtons />
- </Box>
+ {journey?.showChatButtons === true && (
+ <Box>
+ <ChatButtons />
+ </Box>
+ )}
Description
Issue
Accordion
componentSummary by CodeRabbit
- Expanded journey customization with new toggles allowing users to control the visibility of chat buttons, reaction controls, host details, logos, menus, and display titles.
- Enhanced interactive elements and fallback visuals across components for a more intuitive and consistent interface experience.