segments: support adding custom widgets in segment group#1386
segments: support adding custom widgets in segment group#1386tahini merged 1 commit intochairemobilite:mainfrom
Conversation
WalkthroughAdds support for configuring extra segment widgets via a new optional field Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@packages/evolution-common/src/services/questionnaire/sections/segments/__tests__/groupSegments.test.ts`:
- Around line 200-223: The new test 'SegmentsGroupConfigFactory segments
GroupConfig widget with additional widget names' should be moved inside the
existing describe block titled 'SegmentsGroupConfigFactory segments GroupConfig
widget' in groupSegments.test.ts so it follows the same test grouping and setup;
locate the standalone test and cut/paste it into that describe block (keeping
the test name, assertions, and any required setup like segmentSectionConfig and
widgetFactoryOptions), and ensure indentation and imports remain correct after
relocation.
In `@packages/evolution-common/src/services/questionnaire/types/SectionConfig.ts`:
- Around line 508-514: Fix the JSDoc typo for the additionalSegmentWidgetNames
field in SectionConfig by changing "The will be added after the mode question,
before the hasNextMode question." to "They will be added after the mode
question, before the hasNextMode question." so the plural pronoun matches
"widgets"; update the comment block for additionalSegmentWidgetNames in
SectionConfig.ts accordingly.
| test('SegmentsGroupConfigFactory segments GroupConfig widget with additional widget names', () => { | ||
| const segmentSectionConfigWithWidgets: SegmentSectionConfiguration = _cloneDeep(segmentSectionConfig); | ||
| segmentSectionConfigWithWidgets.additionalSegmentWidgetNames = ['customWidget1', 'customWidget2']; | ||
| const widgetConfig = new SegmentsGroupConfigFactory(segmentSectionConfigWithWidgets, widgetFactoryOptions).getWidgetConfigs()['segments'] as GroupConfig; | ||
| expect(widgetConfig).toEqual({ | ||
| type: 'group', | ||
| path: 'segments', | ||
| title: expect.any(Function), | ||
| name: expect.any(Function), | ||
| showTitle: false, | ||
| showGroupedObjectDeleteButton: expect.any(Function), | ||
| showGroupedObjectAddButton: expect.any(Function), | ||
| groupedObjectAddButtonLabel: expect.any(Function), | ||
| addButtonLocation: 'bottom' as const, | ||
| widgets: [ | ||
| 'segmentSameModeAsReverseTrip', | ||
| 'segmentModePre', | ||
| 'segmentMode', | ||
| 'customWidget1', | ||
| 'customWidget2', | ||
| 'segmentHasNextMode' | ||
| ] | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Good test coverage for the new feature.
The test correctly validates widget insertion order. One small nit: this test sits outside the existing describe blocks. Consider nesting it under 'SegmentsGroupConfigFactory segments GroupConfig widget' for consistency.
🤖 Prompt for AI Agents
In
`@packages/evolution-common/src/services/questionnaire/sections/segments/__tests__/groupSegments.test.ts`
around lines 200 - 223, The new test 'SegmentsGroupConfigFactory segments
GroupConfig widget with additional widget names' should be moved inside the
existing describe block titled 'SegmentsGroupConfigFactory segments GroupConfig
widget' in groupSegments.test.ts so it follows the same test grouping and setup;
locate the standalone test and cut/paste it into that describe block (keeping
the test name, assertions, and any required setup like segmentSectionConfig and
widgetFactoryOptions), and ensure indentation and imports remain correct after
relocation.
packages/evolution-common/src/services/questionnaire/types/SectionConfig.ts
Show resolved
Hide resolved
completes chairemobilite#1345 This adds the `additionalSegmentWidgetNames` configuration option to the `SegmentSectionConfiguration` type. It contains a list of widget names to add after the mode question and before the has next mode question. These widgets need to be defined in the survey itself.
bd7ccf3 to
cf7df52
Compare
samuel-duhaime
left a comment
There was a problem hiding this comment.
Any reason why we don't have more control over the widget placement order?
What if I want to put an introduction at the start of the segment section?
| 'segmentModePre', | ||
| 'segmentMode', | ||
| // Additional widgets are added here | ||
| ...additionalWidgetNames, |
There was a problem hiding this comment.
Will the additionalWidgetNames will always be exactly there or we have more control over the placement?
|
The reason is that for the segment group, there is no real need for it. It would complicate how we configure it, I'd rather deal with that problem when it's required. Now, for the group segment, it goes there. For other sections, configurations may be different. |
completes #1345
This adds the
additionalSegmentWidgetNamesconfiguration option to theSegmentSectionConfigurationtype. It contains a list of widget names to add after the mode question and before the has next mode question.These widgets need to be defined in the survey itself.
Summary by CodeRabbit
New Features
Tests
Chores