feat: WCAG 3.3.1 inline error in report message modal #36308
Conversation
🦋 Changeset detectedLatest commit: 3528f39 The changes in this PR will be included in the next version bump. This PR includes changesets to release 42 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Looks like this PR is ready to merge! 🎉 |
gabriellsh
left a comment
gabriellsh
left a comment
There was a problem hiding this comment.
This one is good, just need to fix lint :)
WalkthroughRefactors the report message modal to use react-hook-form Controller, adds i18n and unique IDs, improves accessibility labels/errors; updates E2E page object to a modal-rooted API with toast verification and adapts tests; removes many obsolete report-related translation keys and adds Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Modal as ReportMessageModal
participant Form as react-hook-form (Controller)
participant API as Report API
participant Toast as ToastMessages
rect rgb(248,249,250)
User->>Modal: Open modal
User->>Modal: Type description
User->>Modal: Click "Report"
end
rect rgb(230,247,255)
Modal->>Form: handleSubmit -> Controller supplies description
Form->>API: POST /reports (payload)
API-->>Form: 200 OK
Form-->>Modal: submit handler resolves
Modal->>Toast: show success toast
Toast-->>User: success visible
Modal->>User: dismiss dialog
end
alt Validation error (empty description)
Modal->>Form: handleSubmit -> validation fails
Form-->>Modal: returns errors
Modal->>User: show FieldError (role="alert")
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-09-18T17:32:33.969ZApplied to files:
🔇 Additional comments (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (62)
apps/meteor/client/views/room/modals/ReportMessageModal/ReportMessageModal.tsx(3 hunks)apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts(1 hunks)apps/meteor/tests/e2e/report-message.spec.ts(3 hunks)packages/i18n/src/locales/af.i18n.json(0 hunks)packages/i18n/src/locales/ar.i18n.json(0 hunks)packages/i18n/src/locales/az.i18n.json(0 hunks)packages/i18n/src/locales/be-BY.i18n.json(0 hunks)packages/i18n/src/locales/bg.i18n.json(0 hunks)packages/i18n/src/locales/bs.i18n.json(0 hunks)packages/i18n/src/locales/ca.i18n.json(0 hunks)packages/i18n/src/locales/cs.i18n.json(0 hunks)packages/i18n/src/locales/cy.i18n.json(0 hunks)packages/i18n/src/locales/da.i18n.json(0 hunks)packages/i18n/src/locales/de-AT.i18n.json(0 hunks)packages/i18n/src/locales/de-IN.i18n.json(0 hunks)packages/i18n/src/locales/de.i18n.json(0 hunks)packages/i18n/src/locales/el.i18n.json(0 hunks)packages/i18n/src/locales/en.i18n.json(1 hunks)packages/i18n/src/locales/eo.i18n.json(0 hunks)packages/i18n/src/locales/es.i18n.json(0 hunks)packages/i18n/src/locales/fa.i18n.json(0 hunks)packages/i18n/src/locales/fi.i18n.json(0 hunks)packages/i18n/src/locales/fr.i18n.json(0 hunks)packages/i18n/src/locales/gl.i18n.json(0 hunks)packages/i18n/src/locales/he.i18n.json(0 hunks)packages/i18n/src/locales/hi-IN.i18n.json(0 hunks)packages/i18n/src/locales/hr.i18n.json(0 hunks)packages/i18n/src/locales/hu.i18n.json(0 hunks)packages/i18n/src/locales/id.i18n.json(0 hunks)packages/i18n/src/locales/it.i18n.json(0 hunks)packages/i18n/src/locales/ja.i18n.json(0 hunks)packages/i18n/src/locales/ka-GE.i18n.json(0 hunks)packages/i18n/src/locales/km.i18n.json(0 hunks)packages/i18n/src/locales/ko.i18n.json(0 hunks)packages/i18n/src/locales/ku.i18n.json(0 hunks)packages/i18n/src/locales/lo.i18n.json(0 hunks)packages/i18n/src/locales/lt.i18n.json(0 hunks)packages/i18n/src/locales/lv.i18n.json(0 hunks)packages/i18n/src/locales/mn.i18n.json(0 hunks)packages/i18n/src/locales/ms-MY.i18n.json(0 hunks)packages/i18n/src/locales/nb.i18n.json(0 hunks)packages/i18n/src/locales/nl.i18n.json(0 hunks)packages/i18n/src/locales/nn.i18n.json(0 hunks)packages/i18n/src/locales/pl.i18n.json(0 hunks)packages/i18n/src/locales/pt-BR.i18n.json(0 hunks)packages/i18n/src/locales/pt.i18n.json(0 hunks)packages/i18n/src/locales/ro.i18n.json(0 hunks)packages/i18n/src/locales/ru.i18n.json(0 hunks)packages/i18n/src/locales/sk-SK.i18n.json(0 hunks)packages/i18n/src/locales/sl-SI.i18n.json(0 hunks)packages/i18n/src/locales/sq.i18n.json(0 hunks)packages/i18n/src/locales/sr.i18n.json(0 hunks)packages/i18n/src/locales/sv.i18n.json(0 hunks)packages/i18n/src/locales/ta-IN.i18n.json(0 hunks)packages/i18n/src/locales/th-TH.i18n.json(0 hunks)packages/i18n/src/locales/tr.i18n.json(0 hunks)packages/i18n/src/locales/ug.i18n.json(0 hunks)packages/i18n/src/locales/uk.i18n.json(0 hunks)packages/i18n/src/locales/vi-VN.i18n.json(0 hunks)packages/i18n/src/locales/zh-HK.i18n.json(0 hunks)packages/i18n/src/locales/zh-TW.i18n.json(0 hunks)packages/i18n/src/locales/zh.i18n.json(0 hunks)
💤 Files with no reviewable changes (58)
- packages/i18n/src/locales/de-AT.i18n.json
- packages/i18n/src/locales/el.i18n.json
- packages/i18n/src/locales/be-BY.i18n.json
- packages/i18n/src/locales/th-TH.i18n.json
- packages/i18n/src/locales/tr.i18n.json
- packages/i18n/src/locales/sq.i18n.json
- packages/i18n/src/locales/zh-HK.i18n.json
- packages/i18n/src/locales/ro.i18n.json
- packages/i18n/src/locales/uk.i18n.json
- packages/i18n/src/locales/ka-GE.i18n.json
- packages/i18n/src/locales/zh-TW.i18n.json
- packages/i18n/src/locales/he.i18n.json
- packages/i18n/src/locales/mn.i18n.json
- packages/i18n/src/locales/gl.i18n.json
- packages/i18n/src/locales/bs.i18n.json
- packages/i18n/src/locales/cy.i18n.json
- packages/i18n/src/locales/da.i18n.json
- packages/i18n/src/locales/af.i18n.json
- packages/i18n/src/locales/id.i18n.json
- packages/i18n/src/locales/sr.i18n.json
- packages/i18n/src/locales/pt.i18n.json
- packages/i18n/src/locales/eo.i18n.json
- packages/i18n/src/locales/ca.i18n.json
- packages/i18n/src/locales/sl-SI.i18n.json
- packages/i18n/src/locales/hi-IN.i18n.json
- packages/i18n/src/locales/km.i18n.json
- packages/i18n/src/locales/ta-IN.i18n.json
- packages/i18n/src/locales/vi-VN.i18n.json
- packages/i18n/src/locales/fi.i18n.json
- packages/i18n/src/locales/lv.i18n.json
- packages/i18n/src/locales/ug.i18n.json
- packages/i18n/src/locales/nl.i18n.json
- packages/i18n/src/locales/bg.i18n.json
- packages/i18n/src/locales/ms-MY.i18n.json
- packages/i18n/src/locales/lt.i18n.json
- packages/i18n/src/locales/ar.i18n.json
- packages/i18n/src/locales/az.i18n.json
- packages/i18n/src/locales/cs.i18n.json
- packages/i18n/src/locales/fr.i18n.json
- packages/i18n/src/locales/de-IN.i18n.json
- packages/i18n/src/locales/sv.i18n.json
- packages/i18n/src/locales/es.i18n.json
- packages/i18n/src/locales/hu.i18n.json
- packages/i18n/src/locales/hr.i18n.json
- packages/i18n/src/locales/zh.i18n.json
- packages/i18n/src/locales/ku.i18n.json
- packages/i18n/src/locales/fa.i18n.json
- packages/i18n/src/locales/ru.i18n.json
- packages/i18n/src/locales/ja.i18n.json
- packages/i18n/src/locales/it.i18n.json
- packages/i18n/src/locales/pl.i18n.json
- packages/i18n/src/locales/ko.i18n.json
- packages/i18n/src/locales/nn.i18n.json
- packages/i18n/src/locales/sk-SK.i18n.json
- packages/i18n/src/locales/nb.i18n.json
- packages/i18n/src/locales/de.i18n.json
- packages/i18n/src/locales/pt-BR.i18n.json
- packages/i18n/src/locales/lo.i18n.json
🧰 Additional context used
📓 Path-based instructions (4)
apps/meteor/tests/e2e/page-objects/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Files:
apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts
apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx}: Write concise, technical TypeScript/JavaScript with accurate typing
Follow DRY by extracting reusable logic into helper functions or page objects
Avoid code comments in the implementation
Files:
apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.tsapps/meteor/tests/e2e/report-message.spec.ts
apps/meteor/tests/e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,tsx}: Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Store commonly used locators in variables/constants for reuse
Use page.waitFor() with specific conditions and avoid hardcoded timeouts
Implement proper wait strategies for dynamic content
Follow the Page Object Model pattern consistently
Files:
apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.tsapps/meteor/tests/e2e/report-message.spec.ts
apps/meteor/tests/e2e/**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.spec.ts: All Playwright test files must be located under apps/meteor/tests/e2e/ and use the .spec.ts extension (e.g., login.spec.ts)
Use descriptive test names that clearly communicate expected behavior
Use test.beforeAll() and test.afterAll() for setup and teardown
Use test.step() to organize complex test scenarios
Group related tests in the same file
Utilize Playwright fixtures (test, page, expect) consistently
Prefer web-first assertions (e.g., toBeVisible, toHaveText)
Use expect matchers (toEqual, toContain, toBeTruthy, toHaveLength, etc.) instead of assert statements
Maintain test isolation between test cases
Ensure a clean state for each test execution
Ensure tests run reliably in parallel without shared state conflicts
Files:
apps/meteor/tests/e2e/report-message.spec.ts
🧠 Learnings (14)
📓 Common learnings
Learnt from: aleksandernsilva
Repo: RocketChat/Rocket.Chat PR: 36974
File: apps/meteor/client/components/Omnichannel/OutboundMessage/components/OutboundMessageWizard/forms/MessageForm/MessageForm.tsx:124-129
Timestamp: 2025-09-18T17:32:33.969Z
Learning: The ARIA mismatch issue in MessageForm's template field (where FieldError id used templateId instead of messageFormId) was addressed in PR #36972 through refactoring the template field into a separate TemplateField component, which uses consistent templateFieldId for both aria-describedby and FieldError id.
📚 Learning: 2025-09-18T17:32:33.969Z
Learnt from: aleksandernsilva
Repo: RocketChat/Rocket.Chat PR: 36974
File: apps/meteor/client/components/Omnichannel/OutboundMessage/components/OutboundMessageWizard/forms/MessageForm/MessageForm.tsx:124-129
Timestamp: 2025-09-18T17:32:33.969Z
Learning: The ARIA mismatch issue in MessageForm's template field (where FieldError id used templateId instead of messageFormId) was addressed in PR #36972 through refactoring the template field into a separate TemplateField component, which uses consistent templateFieldId for both aria-describedby and FieldError id.
Applied to files:
apps/meteor/client/views/room/modals/ReportMessageModal/ReportMessageModal.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.tsapps/meteor/tests/e2e/report-message.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.tsapps/meteor/tests/e2e/report-message.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.tsapps/meteor/tests/e2e/report-message.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (e.g., toBeVisible, toHaveText)
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.tsapps/meteor/tests/e2e/report-message.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (test, page, expect) consistently
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.tsapps/meteor/tests/e2e/report-message.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use expect matchers (toEqual, toContain, toBeTruthy, toHaveLength, etc.) instead of assert statements
Applied to files:
apps/meteor/tests/e2e/report-message.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use test.step() to organize complex test scenarios
Applied to files:
apps/meteor/tests/e2e/report-message.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Write concise, technical TypeScript/JavaScript with accurate typing
Applied to files:
apps/meteor/tests/e2e/report-message.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Avoid code comments in the implementation
Applied to files:
apps/meteor/tests/e2e/report-message.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/e2e/report-message.spec.ts
🧬 Code graph analysis (2)
apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts (1)
apps/meteor/tests/e2e/page-objects/fragments/toast-messages.ts (1)
ToastMessages(5-29)
apps/meteor/tests/e2e/report-message.spec.ts (1)
apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(6-120)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
🔇 Additional comments (12)
apps/meteor/tests/e2e/report-message.spec.ts (3)
82-82: LGTM! Empty validation test properly uses new API.The test correctly calls
submitReport()without arguments to verify empty description validation.
99-99: LGTM! Cancel flow properly abstracted.The test correctly uses the new
cancelReport()method to test the cancellation flow.
114-120: LGTM! Successful report submission test updated correctly.The test properly generates a description and passes it to
submitReport(reportDescription)to verify successful report submission.apps/meteor/client/views/room/modals/ReportMessageModal/ReportMessageModal.tsx (7)
3-3: LGTM! Imports updated for accessibility enhancements.The added imports (
FieldLabel,FieldDescription) are necessary for the improved accessibility implementation.
5-9: LGTM! Dependencies updated for form handling and i18n.The updated imports correctly add
useIdfor stable IDs,Controllerfor enhanced form control, anduseTranslationfor i18n integration.
28-38: LGTM! Form setup follows react-hook-form best practices.The form initialization correctly:
- Uses
useTranslationfor i18n- Generates a stable unique ID with
useId()- Includes
controlfor Controller usage- Defines
defaultValuesto prevent uncontrolled component warnings
68-69: LGTM! Proper label and description for accessibility.The
FieldLabelwithhtmlForandFieldDescriptionwith a unique ID correctly link the label and description to the input field for screen readers.
71-85: Excellent ARIA implementation for WCAG 3.3.1 compliance.The Controller implementation includes proper accessibility attributes:
aria-required='true'indicates the field is requiredaria-invaliddynamically reflects validation statearia-describedbylinks to both description and error (when present)id={reasonForReportId}connects to the label'shtmlForThis properly implements WCAG 3.3.1 (Error Identification) requirements.
87-91: LGTM! Error rendering with proper accessibility.The error message correctly:
- Uses
role='alert'to announce errors to screen readers- Has a unique
idthat matches thearia-describedbyreference- Displays the dynamic validation message from
errors.description.message
59-61: All translation keys verified and exist in locale files.The verification is complete. Both
Report_messageandReporttranslation keys have been confirmed to exist across the i18n locale files, including the English locale (en.i18n.json). The code changes are properly supported by the translation infrastructure.packages/i18n/src/locales/en.i18n.json (1)
4374-4376: ****The code is already correct. The ReportMessageModal.tsx at line 59 correctly references
t('Report_message')which matches the key defined in en.i18n.json. No mismatch exists, and old report keys are not referenced anywhere in the codebase, confirming they were successfully removed.Likely an incorrect or invalid review comment.
apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts (1)
7-47: Well-structured page object with clean API design.The refactoring to extend Modal and encapsulate actions in
submitReport()andcancelReport()methods follows the Page Object Model pattern effectively. The integration with ToastMessages for verification and the dual-path submitReport (validation vs. full submission) provides a flexible testing API.
apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts
Outdated
Show resolved
Hide resolved
dougfabris
force-pushed
the
fix/inline-error-report-message-modal
branch
from
b05ebdc to
e924679
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts (1)
16-16: Past redundant dialog selector issue has been resolved.These locators now correctly query directly from
this.rootwithout the redundant.getByRole('dialog')chain mentioned in the previous review.Also applies to: 20-20, 24-24
🧹 Nitpick comments (1)
apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts (1)
36-47: Consider splitting dual-behavior method for clarity.The
submitReportmethod serves two purposes: validation testing (no args) and successful submission (with description). While functional, this dual behavior may be less intuitive for test authors.Consider extracting the validation scenario into a separate method:
+async triggerValidation(): Promise<void> { + await this.btnSubmitReport.click(); + await expect(this.alertInputDescription).toBeVisible(); +} + async submitReport(description: string): Promise<void> { - if (!description) { - await this.btnSubmitReport.click(); - await expect(this.alertInputDescription).toBeVisible(); - return; - } - await this.inputReportDescription.fill(description); await this.btnSubmitReport.click(); await this.waitForDismissal(); await this.toastMessage.waitForDisplay({ type: 'success', message: 'Report has been sent' }); }Then update the test at line 82 in
report-message.spec.ts:-await reportModal.submitReport(); +await reportModal.triggerValidation();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts(1 hunks)apps/meteor/tests/e2e/report-message.spec.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/meteor/tests/e2e/**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.spec.ts: All Playwright test files must be located under apps/meteor/tests/e2e/ and use the .spec.ts extension (e.g., login.spec.ts)
Use descriptive test names that clearly communicate expected behavior
Use test.beforeAll() and test.afterAll() for setup and teardown
Use test.step() to organize complex test scenarios
Group related tests in the same file
Utilize Playwright fixtures (test, page, expect) consistently
Prefer web-first assertions (e.g., toBeVisible, toHaveText)
Use expect matchers (toEqual, toContain, toBeTruthy, toHaveLength, etc.) instead of assert statements
Maintain test isolation between test cases
Ensure a clean state for each test execution
Ensure tests run reliably in parallel without shared state conflicts
Files:
apps/meteor/tests/e2e/report-message.spec.ts
apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx}: Write concise, technical TypeScript/JavaScript with accurate typing
Follow DRY by extracting reusable logic into helper functions or page objects
Avoid code comments in the implementation
Files:
apps/meteor/tests/e2e/report-message.spec.tsapps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts
apps/meteor/tests/e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,tsx}: Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Store commonly used locators in variables/constants for reuse
Use page.waitFor() with specific conditions and avoid hardcoded timeouts
Implement proper wait strategies for dynamic content
Follow the Page Object Model pattern consistently
Files:
apps/meteor/tests/e2e/report-message.spec.tsapps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts
apps/meteor/tests/e2e/page-objects/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Files:
apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts
🧠 Learnings (15)
📓 Common learnings
Learnt from: aleksandernsilva
Repo: RocketChat/Rocket.Chat PR: 36974
File: apps/meteor/client/components/Omnichannel/OutboundMessage/components/OutboundMessageWizard/forms/MessageForm/MessageForm.tsx:124-129
Timestamp: 2025-09-18T17:32:33.969Z
Learning: The ARIA mismatch issue in MessageForm's template field (where FieldError id used templateId instead of messageFormId) was addressed in PR #36972 through refactoring the template field into a separate TemplateField component, which uses consistent templateFieldId for both aria-describedby and FieldError id.
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/tests/e2e/report-message.spec.tsapps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Write concise, technical TypeScript/JavaScript with accurate typing
Applied to files:
apps/meteor/tests/e2e/report-message.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use expect matchers (toEqual, toContain, toBeTruthy, toHaveLength, etc.) instead of assert statements
Applied to files:
apps/meteor/tests/e2e/report-message.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (e.g., toBeVisible, toHaveText)
Applied to files:
apps/meteor/tests/e2e/report-message.spec.tsapps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (test, page, expect) consistently
Applied to files:
apps/meteor/tests/e2e/report-message.spec.tsapps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Avoid code comments in the implementation
Applied to files:
apps/meteor/tests/e2e/report-message.spec.tsapps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Applied to files:
apps/meteor/tests/e2e/report-message.spec.tsapps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Applied to files:
apps/meteor/tests/e2e/report-message.spec.tsapps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/e2e/report-message.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/tests/e2e/report-message.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Follow DRY by extracting reusable logic into helper functions or page objects
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts
📚 Learning: 2025-09-23T19:22:59.217Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 36987
File: apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts:10-20
Timestamp: 2025-09-23T19:22:59.217Z
Learning: In Playwright e2e tests, prefer stable selectors like data-qa-id attributes over localized text in getByRole() or getByText() calls to prevent test failures when UI language changes. Test translations separately by validating actual text content after ensuring UI interactions work with stable selectors.
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts
🧬 Code graph analysis (2)
apps/meteor/tests/e2e/report-message.spec.ts (1)
apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(6-120)
apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts (1)
apps/meteor/tests/e2e/page-objects/fragments/toast-messages.ts (1)
ToastMessages(5-29)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ⚙️ Variables Setup
- GitHub Check: CodeQL-Build
🔇 Additional comments (4)
apps/meteor/tests/e2e/page-objects/fragments/report-message-modal.ts (1)
7-13: Good refactor to extend Modal and integrate toast verification.Extending the base
Modalclass and adding thetoastMessageproperty improves reusability and allows verification of success messages after submission.apps/meteor/tests/e2e/report-message.spec.ts (3)
82-82: Validation test correctly uses the new modal API.Calling
submitReport()without arguments triggers the validation path in the modal, which clicks submit and verifies the alert is displayed.
99-99: Good use of encapsulated cancelReport method.The new helper method improves readability and maintainability by hiding the implementation details of canceling the report flow.
114-120: Success scenario properly tests the full report flow.The test correctly generates a description, passes it to
submitReport, and the modal handles filling the input, submitting, waiting for dismissal, and verifying the success toast.
dougfabris
changed the title
dougfabris
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
5 participants
Proposed changes (including videos or screenshots)
This PR improves the accessibility and UX consistency of the Report Message Modal
Changes made:
useId()andaria-describedbyReport_Messagekey toen.i18n.jsonReportUserModalIssue(s)
Steps to test or reproduce
Before:
After:
Further comments
WA-52
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
Documentation