enhance(apps/frontend-manage): add completeness states to workflow component in wizards #4365
Conversation
Current Aviator status
This PR was merged manually (without Aviator). Merging manually can negatively impact the performance of the queue. Consider using Aviator next time.
See the real-time status of this PR on the
Aviator webapp.
Use the Aviator Chrome Extension
to see the status of your PR within GitHub.
|
📝 WalkthroughWalkthroughThe pull request includes updates to the Changes
Possibly related PRs
Suggested reviewers
Warning Tool Failures:Tool Failure Count:Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (9)
apps/frontend-manage/src/components/sessions/creation/liveQuiz/LiveSessionWizard.tsx (3)
76-78: Consider more granular step validation initialization.While the explicit typing of
stepValidityasboolean[]improves type safety, initializing all steps as valid in edit mode (!!initialValues) might not accurately reflect the actual validity state of each step. Consider initializing each step's validity based on its specific validation requirements.-const [stepValidity, setStepValidity] = useState<boolean[]>( - Array(4).fill(!!initialValues) -) +const [stepValidity, setStepValidity] = useState<boolean[]>(() => { + if (!initialValues) return Array(4).fill(false); + return [ + nameValidationSchema.isValidSync(initialValues), + descriptionValidationSchema.isValidSync(initialValues), + settingsValidationSchema.isValidSync(initialValues), + questionsValidationSchema.isValidSync(initialValues), + ]; +});
161-161: Consider using an enum for step indices.The completed property correctly references stepValidity indices, but using magic numbers (0,1,2,3) makes the code less maintainable. Consider introducing an enum to manage step indices.
enum WizardSteps { INFORMATION = 0, DESCRIPTION = 1, SETTINGS = 2, QUESTIONS = 3, } const workflowItems = [ { title: t('shared.generic.information'), completed: stepValidity[WizardSteps.INFORMATION], }, // ... other items using WizardSteps enum ];Also applies to: 167-167, 173-173, 179-179
Line range hint
1-359: Consider architectural improvements for better maintainability.The component handles complex functionality well but could benefit from the following improvements:
- Move validation schemas to a separate file to improve maintainability
- Implement lazy loading for step components to improve initial load time
- Consider extracting the wizard logic into a custom hook for reusability
Example implementation for lazy loading:
const LiveQuizInformationStep = lazy(() => import('./LiveQuizInformationStep')); const LiveQuizDescriptionStep = lazy(() => import('./LiveQuizDescriptionStep')); // ... other step components // Wrap the steps with Suspense <Suspense fallback={<LoadingSpinner />}> {steps[activeStep]} </Suspense>Would you like me to help create a separate PR for these improvements?
apps/frontend-manage/src/components/sessions/creation/practiceQuiz/PracticeQuizWizard.tsx (2)
95-97: Consider deriving stepValidity length from workflow steps.While the explicit typing improves type safety, the hardcoded array length of 4 could become a maintenance issue if steps are added or removed.
Consider this more maintainable approach:
-const [stepValidity, setStepValidity] = useState<boolean[]>( - Array(4).fill(!!initialValues) -) +const WORKFLOW_STEPS = ['information', 'description', 'settings', 'questions'] as const +const [stepValidity, setStepValidity] = useState<boolean[]>( + Array(WORKFLOW_STEPS.length).fill(!!initialValues) +)
Line range hint
1-424: Consider architectural improvements for better maintainability.The component is well-structured but could benefit from the following improvements:
- Move validation schemas to a separate file to improve maintainability and reusability
- Consider lazy loading step components to improve initial load performance
Example implementation for validation schemas:
// validationSchemas.ts export const nameValidationSchema = yup.object().shape({ name: yup.string().required(t('manage.sessionForms.sessionName')), }) // ... other schemasExample implementation for lazy loading:
const PracticeQuizInformationStep = lazy(() => import('./PracticeQuizInformationStep')) const PracticeQuizDescriptionStep = lazy(() => import('./PracticeQuizDescriptionStep')) // ... other stepsapps/frontend-manage/src/components/sessions/creation/microLearning/MicroLearningWizard.tsx (2)
82-84: Consider deriving stepValidity initialization from workflowItems.While the explicit typing improves type safety, using a magic number (4) for array initialization makes the code brittle. Consider deriving the array length from workflowItems to maintain consistency.
- const [stepValidity, setStepValidity] = useState<boolean[]>( - Array(4).fill(!!initialValues) - ) + const [stepValidity, setStepValidity] = useState<boolean[]>(() => + new Array(workflowItems.length).fill(!!initialValues) + )
193-213: Consider making the step-validity relationship more explicit.The mapping between stepValidity indices and workflowItems is implicit. Consider using an enum or constants to make this relationship more maintainable and type-safe.
// Add at the top of the file enum WizardStep { INFORMATION = 0, DESCRIPTION = 1, SETTINGS = 2, QUESTIONS = 3, } // Then in workflowItems const workflowItems = [ { title: t('shared.generic.information'), tooltip: t('manage.sessionForms.microLearningInformation'), completed: stepValidity[WizardStep.INFORMATION], }, // ... rest of the items using WizardStep enum ]apps/frontend-manage/src/components/sessions/creation/groupActivity/GroupActivityWizard.tsx (2)
84-86: LGTM! Consider const assertion for array length.The explicit typing and initialization logic look good. For additional type safety, consider defining the number of steps as a const:
+const TOTAL_WIZARD_STEPS = 4 as const; const [stepValidity, setStepValidity] = useState<boolean[]>( - Array(4).fill(!!initialValues) + Array(TOTAL_WIZARD_STEPS).fill(!!initialValues) )
222-239: Enhance type safety for workflow items completion states.While the completion states are correctly implemented, consider these improvements for better type safety and maintainability:
- Create a type for workflow items
- Add runtime validation to ensure stepValidity length matches workflow items
- Use tuple type for stepValidity to ensure exact length
type WorkflowItem = { title: string tooltip: string tooltipDisabled?: string completed: boolean } // Ensure exact length with tuple type const [stepValidity, setStepValidity] = useState<[boolean, boolean, boolean, boolean]>( Array(4).fill(!!initialValues) as [boolean, boolean, boolean, boolean] ) // Add runtime validation if (workflowItems.length !== stepValidity.length) { console.error('Workflow items length does not match step validity length') }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
apps/auth/package.json(1 hunks)apps/backend-docker/package.json(1 hunks)apps/docs/package.json(1 hunks)apps/frontend-control/package.json(1 hunks)apps/frontend-manage/package.json(1 hunks)apps/frontend-manage/src/components/sessions/creation/groupActivity/GroupActivityWizard.tsx(2 hunks)apps/frontend-manage/src/components/sessions/creation/liveQuiz/LiveSessionWizard.tsx(2 hunks)apps/frontend-manage/src/components/sessions/creation/microLearning/MicroLearningWizard.tsx(2 hunks)apps/frontend-manage/src/components/sessions/creation/practiceQuiz/PracticeQuizWizard.tsx(2 hunks)apps/frontend-pwa/package.json(1 hunks)apps/func-incoming-responses/package.json(1 hunks)apps/func-response-processor/package.json(1 hunks)apps/office-addin/package.json(1 hunks)packages/markdown/package.json(1 hunks)packages/shared-components/package.json(1 hunks)packages/transactional/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (8)
- apps/auth/package.json
- apps/backend-docker/package.json
- apps/frontend-control/package.json
- apps/frontend-pwa/package.json
- apps/func-incoming-responses/package.json
- apps/func-response-processor/package.json
- packages/markdown/package.json
- packages/transactional/package.json
🔇 Additional comments (6)
apps/docs/package.json (1)
24-24: LGTM! Verify documentation updates.
The dependency update to @uzh-bf/design-system@3.0.0-alpha.34 aligns with the PR's objective of enhancing workflow components.
Let's verify if the documentation for the new workflow completeness states exists:
packages/shared-components/package.json (1)
38-38: Version update looks consistent, but verify alpha version changes.
The update of @uzh-bf/design-system from 3.0.0-alpha.32 to 3.0.0-alpha.34 aligns with the PR objectives for enhancing workflow components. However, since this is an alpha version:
- Ensure you've reviewed the changelog between alpha.32 and alpha.34 for any breaking changes
- Consider setting up a migration plan to a stable release when available
Let's verify the consistency of this update across the monorepo:
✅ Verification successful
Version update is consistently applied across all packages ✓
The verification confirms that @uzh-bf/design-system version 3.0.0-alpha.34 is consistently used across all package.json files in the monorepo:
- 3 packages: transactional, shared-components, markdown
- 9 apps: office-addin, func-response-processor, backend-docker, func-incoming-responses, frontend-pwa, auth, frontend-control, docs, frontend-manage
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent @uzh-bf/design-system version across all package.json files
# Expected: All occurrences should show version 3.0.0-alpha.34
# Search for @uzh-bf/design-system version declarations
rg -A 1 '"@uzh-bf/design-system":'
Length of output: 1780
apps/office-addin/package.json (1)
6-6: Verify compatibility with the design system update.
The update to @uzh-bf/design-system@3.0.0-alpha.34 looks good. However, since this is an alpha version update that includes workflow component changes:
- Please verify that the new workflow completeness states work correctly in your Office Add-in implementation
- Given your IE 11 support requirement, ensure the new features are compatible with legacy browsers
Let's check the changelog or commits for potential breaking changes:
apps/frontend-manage/package.json (2)
24-24: Verify breaking changes in design system upgrade.
Since this is an alpha version upgrade, please review the changelog of @uzh-bf/design-system for any breaking changes between alpha.32 and alpha.34, particularly around workflow components.
Let's check for a changelog:
#!/bin/bash
# Description: Look for changelog entries in design system
# Expected: Find changelog entries for versions alpha.32 to alpha.34
# Search for changelog files
fd -i changelog
# If not found directly, check README or docs
fd -e md -e mdx | xargs rg -i "changelog|breaking changes"24-24: LGTM! Verify version consistency across workspace.
The version bump of @uzh-bf/design-system to 3.0.0-alpha.34 aligns with the PR objective to enhance workflow components.
Let's verify that all workspace packages use the same version:
✅ Verification successful
Version consistency verified across workspace
All packages in the workspace are using the same version 3.0.0-alpha.34 of @uzh-bf/design-system. The version bump is consistent across:
- All frontend apps (manage, control, pwa)
- Backend services and functions
- Shared packages (components, markdown, transactional)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for version consistency of @uzh-bf/design-system across workspace
# Expected: All occurrences should show version 3.0.0-alpha.34
rg '"@uzh-bf/design-system": "([^"]+)"' -g 'package.json'
Length of output: 460
Script:
#!/bin/bash
# Let's try again with ripgrep to find @uzh-bf/design-system dependencies
# Using a different pattern that's more specific
rg -U '@uzh-bf/design-system.*"([0-9]+\.[0-9]+\.[0-9]+[^"]*)"' --type json
Length of output: 1041
apps/frontend-manage/src/components/sessions/creation/practiceQuiz/PracticeQuizWizard.tsx (1)
200-217: LGTM! Well-structured workflow step definitions.
The addition of the completed property to each workflow item, mapped to stepValidity indices, provides a clean way to track step completion. The tooltip system enhances user experience by providing clear feedback on both enabled and disabled states.
klicker-uzh
|
||||||||||||||||||||||||||||
| Project |
klicker-uzh
|
| Branch Review |
wizard-workflow-states
|
| Run status |
|
| Run duration | 11m 54s |
| Commit |
|
| Committer | Julius Schlapbach |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
140
|
| View all changes introduced in this branch ↗︎ | |
klicker-uzh
|
||||||||||||||||||||||||||||
| Project |
klicker-uzh
|
| Branch Review |
v3
|
| Run status |
|
| Run duration | 11m 22s |
| Commit |
|
| Committer | Julius Schlapbach |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
Failures
|
0
|
|
Flaky
|
0
|
|
Pending
|
0
|
|
Skipped
|
0
|
|
Passing
|
140
|
| View all changes introduced in this branch ↗︎ | |
Summary by CodeRabbit
New Features
@uzh-bf/design-systemdependency to version3.0.0-alpha.34across multiple applications and packages, potentially introducing new features and improvements.Bug Fixes
Documentation