Enabled auto-switch feature for DOB input on the Patient Registration page #9870
Conversation
WalkthroughThe pull request focuses on enhancing the Date of Birth (DOB) input functionality in the Patient Registration component. The changes implement an auto-focus feature that automatically transitions between day, month, and year input fields when a valid two-digit input is entered. This improvement aims to streamline the user experience by reducing manual tab or click interactions when filling out the DOB fields. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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 (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![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 comments (1)
src/components/Patient/PatientRegistration.tsx (1)
Replace custom date inputs with the existing
DateTextInputcomponentThe codebase already has a
DateTextInputcomponent insrc/components/Common/DateTextInput.tsxthat properly handles keyboard navigation (arrow keys, backspace/delete) while maintaining the auto-focus functionality. Using this component would improve accessibility and maintain consistency across the application.🔗 Analysis chain
Line range hint
501-553: Consider keyboard navigation accessibility.While the auto-switch feature improves UX, ensure it doesn't interfere with keyboard navigation (Tab/Shift+Tab). Users who prefer keyboard navigation should still be able to move backwards through fields.
Run this check to verify keyboard event handling:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for keyboard event handlers in the component # Look for existing keyboard event handlers rg -A 2 "onKeyDown|onKeyPress|onKeyUp"Length of output: 1414
Script:
#!/bin/bash # Check the DateTextInput component implementation cat src/components/Common/DateTextInput.tsxLength of output: 7195
🧹 Nitpick comments (1)
src/components/Patient/PatientRegistration.tsx (1)
Line range hint
542-553: Refactor date handling logic for better maintainability.The date string manipulation is duplicated across all three handlers. Consider extracting this logic into a utility function.
Consider this implementation:
+const updateDateOfBirth = ( + current: string | undefined, + part: 'year' | 'month' | 'day', + value: string +) => { + const [year, month, day] = (current || '--').split('-'); + return { + year: part === 'year' ? value : year, + month: part === 'month' ? value : month, + day: part === 'day' ? value : day, + }; +}; // In the component onChange={(e) => { - setForm((f) => ({ - ...f, - date_of_birth: `${e.target.value}-${form.date_of_birth?.split("-")[1] || ""}-${form.date_of_birth?.split("-")[2] || ""}`, - })) + const { year, month, day } = updateDateOfBirth(form.date_of_birth, 'year', e.target.value); + setForm((f) => ({ + ...f, + date_of_birth: `${year}-${month}-${day}`, + })); }}
…aan/care_feAmaan into issues/9869/DOB-autoswitch
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cypress/support/commands.ts (2)
180-186: Remove commented-out code.The old implementation should be removed as it's no longer needed and can be retrieved from version control if necessary. Keeping commented-out code adds noise and can lead to confusion during maintenance.
193-193: Consider adding a dedicated command for DOB input testing.Since this PR implements an auto-switch feature for DOB input fields, consider adding a specialized command to streamline DOB-related tests. This would improve test maintainability and readability.
Example implementation:
Cypress.Commands.add('typeDateOfBirth', (day: string, month: string, year: string) => { // Assuming the fields have data-testid attributes cy.get('[data-testid="dob-day"]').type(day); cy.get('[data-testid="dob-month"]').type(month); cy.get('[data-testid="dob-year"]').type(year); // Verify auto-switch behavior if (day.length === 2) { cy.get('[data-testid="dob-month"]').should('be.focused'); } if (month.length === 2) { cy.get('[data-testid="dob-year"]').should('be.focused'); } });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cypress/support/commands.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (1)
cypress/support/commands.ts (1)
191-193: LGTM! Great improvements to the test command.The new implementation is more maintainable and follows Cypress best practices by:
- Using aliases for better readability
- Breaking down the operation into clear steps
- Making the element interaction flow more explicit
|
@rithviknishad check the updates |
|
👋 Hi, @modamaan, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
…aan/care_feAmaan into issues/9869/DOB-autoswitch
| @@ -507,19 +514,27 @@ export default function PatientRegistration( | |||
| <span className="text-red-500">*</span> | |||
| </Label> | |||
| <Input | |||
| id="month-input" | |||
There was a problem hiding this comment.
let's keep the id bit more specific
| id="month-input" | |
| id="dob-month-input" |
Proposed Changes
loom-video.8.mp4
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit