feat(web): WHODAS calculator #16947
feat(web): WHODAS calculator #16947
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces a new WHODAS calculator component to the web application. It includes a CSS module for styling, a TypeScript file for internationalized strings, and the main React component that manages user input and displays results. The changes enhance the user interface and functionality of the WHODAS calculator, allowing for better user interaction and localization. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
Documentation and Community
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16947 +/- ##
==========================================
- Coverage 36.41% 36.39% -0.02%
==========================================
Files 6899 6901 +2
Lines 144866 144941 +75
Branches 41402 41422 +20
==========================================
Hits 52756 52756
- Misses 92110 92185 +75
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 5 Total Test Services: 0 Failed, 5 Passed Test Services
🔻 Code Coverage Decreases vs Default Branch (1)
|
RunarVestmann
added
the
deprecated:automerge
![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
🧹 Outside diff range and nitpick comments (7)
apps/web/components/connected/WHODAS/Calculator.strings.ts (2)
4-25: Enhance type safety and translator context.Two suggestions for improvement:
- Add type information for the progress message placeholders
- Provide more descriptive context in the description fields for translators
Apply this diff to improve the code:
progress: { id: 'web.whodas.calculator:form.progress', defaultMessage: 'Skref {stepIndex} af {stepCount}', - description: 'Skref {stepIndex} af {stepCount}', + description: 'Progress indicator showing current step number out of total steps', + values: { + stepIndex: { + type: 'number', + minimum: 1, + }, + stepCount: { + type: 'number', + minimum: 1, + }, + }, },
26-52: Consider using an enum for answer labels.The current implementation uses string keys for numeric values. Consider using an enum to improve type safety and maintainability.
Create a new type definition:
enum DifficultyLevel { None = '0', Slight = '1', Moderate = '2', Severe = '3', Extreme = '4', } // Then use it in the messages: export const m = { answerLabel: defineMessages<Record<DifficultyLevel, MessageDescriptor>>({ [DifficultyLevel.None]: { // ... existing message }, // ... other messages }), }apps/web/utils/richText.tsx (1)
203-205: Consider improving type safety for component mappingWhile the implementation is correct, consider enhancing type safety by:
- Defining a union type for all component types
- Ensuring the slice prop type is properly constrained
Example implementation:
type ComponentType = | 'Fiskistofa/ShipSearch' | 'WHODAS/Calculator' // ... other types interface WHODASSlice extends ConnectedComponent { // Add specific WHODAS calculator props here } // In the switch statement case 'WHODAS/Calculator': connectedComponent = <WHODASCalculator slice={slice as WHODASSlice} /> break;apps/web/components/connected/WHODAS/Calculator.tsx (4)
41-49: Enhance type safety by reusing existing interfacesThe
CheckboxStateinterface defines structures similar to the existingStepandQuestioninterfaces. Reusing these interfaces can improve type consistency and reduce code duplication.Consider modifying the
CheckboxStateinterface as follows:-interface CheckboxState { - steps: { - title: string - questions: { - selectedAnswerIndex: number - answerScore: number - }[] - }[] -} +interface QuestionState extends Omit<Question, 'answerOptions'> { + selectedAnswerIndex: number + answerScore: number +} + +interface StepState extends Omit<Step, 'questions'> { + questions: QuestionState[] +} + +interface CheckboxState { + steps: StepState[] +}This approach reuses the properties from
QuestionandStep, adding only the necessary state-related fields.
263-266: Avoid using hardcoded values for better maintainabilityUsing hardcoded values like
16.9can make future updates error-prone. Consider extracting this value into a constant or retrieving it from configuration to enhance flexibility and readability.Apply this diff to define a constant for the breakpoint:
+const FIRST_BRACKET_BREAKPOINT = slice.configJson?.firstBracketBreakpoint ?? 16.9 return ( <WHODASResults results={results} bracket={ - totalScore <= (slice.configJson?.firstBracketBreakpoint ?? 16.9) + totalScore <= FIRST_BRACKET_BREAKPOINT ? 1 : 2 } /> )
81-83: Clarify the purpose of limiting answer options to four choicesThe condition
if (answerIndex > 3)filters out answer options beyond the first four. Ensure this aligns with the intended design and consider adding a comment to explain this limitation for future maintainability.Add a comment explaining why only the first four answer options are considered:
if (answerIndex > 3) { + // Only display the first four answer options as per the survey design return null }
272-317: Enhance accessibility by adding ARIA labels to navigation buttonsTo improve accessibility, consider adding ARIA labels to the "Next" and "Previous" buttons to provide better context for screen readers.
Apply this diff to include
aria-labelattributes:<Button size="small" variant="ghost" preTextIcon="arrowBack" + aria-label="Previous Step" onClick={() => { setStepIndex((s) => s - 1) }} > {formatMessage(m.form.previousStep)} </Button> <Button size="small" + aria-label="Next Step" onClick={() => { setStepIndex((s) => s + 1) }} > {formatMessage( stepIndex >= steps.length - 1 ? m.form.seeResults : m.form.nextStep, )} </Button>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
apps/web/components/connected/WHODAS/Calculator.css.ts(1 hunks)apps/web/components/connected/WHODAS/Calculator.strings.ts(1 hunks)apps/web/components/connected/WHODAS/Calculator.tsx(1 hunks)apps/web/utils/richText.tsx(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/components/connected/WHODAS/Calculator.css.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/web/components/connected/WHODAS/Calculator.strings.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/components/connected/WHODAS/Calculator.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/utils/richText.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (6)
apps/web/components/connected/WHODAS/Calculator.strings.ts (2)
1-3: LGTM! Clean and well-structured initialization.
The file follows react-intl best practices with proper typing through defineMessages.
89-98: Verify markdown processing for advice messages.
The advice messages are marked with #markdown suffix in their IDs, but there's no visible markdown processing setup.
apps/web/utils/richText.tsx (1)
91-91: LGTM! Import follows established patterns
The WHODASCalculator import follows the project's component organization and import conventions.
apps/web/components/connected/WHODAS/Calculator.tsx (3)
215-244: Verify that scrolling behavior works as intended on all devices
The useEffect hook scrolls to the top of the form on step changes, except during the initial render. Ensure this behavior provides a good user experience across different devices and does not interfere with browser accessibility features.
Test the scrolling functionality on various devices and screen sizes to confirm that it enhances, rather than hinders, the user experience.
136-145: Confirm print functionality works correctly across browsers
The window.print() method is used to print the results. Verify that this feature works seamlessly across different browsers and devices, and that the printed output is formatted appropriately.
Test the print functionality in various browsers to ensure consistent behavior and acceptable print layout.
85-89: Ensure safe access to localization messages to prevent runtime errors
Accessing dynamic keys in localization objects could result in undefined if the key does not exist. Confirm that all possible answerIndex values have corresponding entries in m.answerLabel.
Run the following script to verify that all indices up to 3 have corresponding localization entries:
✅ Verification successful
Let me search for the message definitions to understand how these labels are actually defined.
The localization messages are properly defined and safe to use
The code is actually safe as all required keys (0 to 4) are properly defined in Calculator.strings.ts. The type assertion as keyof typeof m.answerLabel ensures type safety, and the actual implementation shows that all possible answer indices (0 to 4) have corresponding localization messages defined.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check that m.answerLabel has entries for keys '0' to '3'
# Expected: All keys from '0' to '3' exist in m.answerLabel
for i in {0..3}; do
rg "m\.answerLabel\['$i'\]" ./ -l || echo "Key '$i' is missing in m.answerLabel"
done
Length of output: 583
Script:
#!/bin/bash
# Search for the message definitions related to answerLabel
rg "answerLabel" -B 2 -A 2
Length of output: 2932
RunarVestmann
force-pushed
the
feature/web-whodas-calculator
branch
from
ff9eec4 to
1a20933
Compare
* WIP * Use dynamic text * Add translation * Add progress meter * Only scroll when not initially rendering * Update id * Style links --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
WHODAS calculator
Screenshots / Gifs
Screen.Recording.2024-11-20.at.16.10.02.mov
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Style
Chores