chore: Use FieldHint instead of FieldDescription on create modals
#36909
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughReplaces FieldDescription with FieldHint in channel/team creation modals. Adds Storybook stories for both modals and story-driven Jest + axe accessibility tests. Initializes Meteor runtime config in RouterContextMock for stories. No changes to exported APIs or business logic. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Jest as Jest Runner
participant Stories as composeStories(...)
participant Wrapper as mockAppRoot().build()
participant Story as Story Component
participant Axe as jest-axe
Jest->>Stories: Load all Create*Modal stories
loop For each story
Jest->>Wrapper: Render(Story)
Wrapper->>Story: Provide app/router context
Note over Story: Renders Create*Modal (V1 or V2)
Jest->>Axe: axe(container)
Axe-->>Jest: a11y results
Jest->>Jest: Snapshot baseElement
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
✨ Finishing touches🧪 Generate unit tests
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #36909 +/- ##
===========================================
+ Coverage 66.46% 66.53% +0.06%
===========================================
Files 3342 3344 +2
Lines 114309 114626 +317
Branches 21080 21194 +114
===========================================
+ Hits 75980 76268 +288
- Misses 35647 35663 +16
- Partials 2682 2695 +13
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![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
🧹 Nitpick comments (7)
apps/meteor/client/stories/contexts/RouterContextMock.tsx (1)
12-16: Don’t clobber global Meteor runtime config; guard and keep typesUnconditional assignment can overwrite existing config in story/test runs; also the property isn’t typed.
Apply within this hunk:
-// Ensure Meteor settings are defined -window.__meteor_runtime_config__ = { - ROOT_URL: 'http://localhost:3000', - ROOT_URL_PATH_PREFIX: '', -}; +// Ensure Meteor settings are defined (preserve existing values) +(window as any).__meteor_runtime_config__ = { + ...((window as any).__meteor_runtime_config__ ?? {}), + ROOT_URL: (window as any).__meteor_runtime_config__?.ROOT_URL ?? 'http://localhost:3000', + ROOT_URL_PATH_PREFIX: (window as any).__meteor_runtime_config__?.ROOT_URL_PATH_PREFIX ?? '', +};Outside this hunk, consider adding a global type to avoid casts:
declare global { interface Window { __meteor_runtime_config__?: { ROOT_URL?: string; ROOT_URL_PATH_PREFIX?: string; [k: string]: unknown; }; } }apps/meteor/client/sidebar/header/CreateTeam/CreateTeamModal.spec.tsx (1)
1-25: Solid story-driven smoke + a11y tests; add a couple of guardrails
- Ensure jest-axe matchers are registered globally (expect.extend from 'jest-axe/extend-expect'); otherwise toHaveNoViolations will fail.
- Snapshot of baseElement can be brittle due to dynamic ids; consider targeted assertions for key UI bits.
apps/meteor/client/sidebar/header/CreateChannel/CreateChannelModal.spec.tsx (1)
1-5: Story-driven tests — LGTM; watch for minor flakiness and naming drift
- Same note on jest-axe matcher registration and snapshot brittleness as above.
- The stories file has a “CreateChanelModal” typo per repo patterns; verify story import names to avoid confusion in CI.
Also applies to: 7-7, 16-28
apps/meteor/client/sidebar/header/CreateTeam/CreateTeamModal.stories.tsx (2)
6-9: Provide default args for required propsCreateTeamModal requires onClose; setting a no-op avoids runtime issues in stories/tests.
export default { title: 'views/sidebar/CreateTeamModal', component: CreateTeamModal, + args: { + onClose: () => undefined, + }, } satisfies Meta<typeof CreateTeamModal>;
11-12: Mirror args on V2 story for consistencyNot strictly required, but keeping both stories consistent helps avoid future regressions.
-export const Default: StoryFn<typeof CreateTeamModal> = (args) => <CreateTeamModal {...args} />; -export const DefaultVersion2: StoryFn<typeof CreateTeamModalV2> = (args) => <CreateTeamModalV2 {...args} />; +export const Default: StoryFn<typeof CreateTeamModal> = (args) => <CreateTeamModal {...args} />; +export const DefaultVersion2: StoryFn<typeof CreateTeamModalV2> = (args) => <CreateTeamModalV2 {...args} />; +Default.args = { onClose: () => undefined }; +DefaultVersion2.args = { onClose: () => undefined };apps/meteor/client/sidebar/header/CreateChannel/CreateChannelModal.stories.tsx (2)
3-3: Fix “Channel” typo across import, meta, and story.Consistently use “CreateChannelModal” to avoid confusion in Storybook navigation and search.
-import CreateChanelModal from './CreateChannelModal'; +import CreateChannelModal from './CreateChannelModal';export default { - title: 'views/sidebar/CreateChanelModal', - component: CreateChanelModal, -} satisfies Meta<typeof CreateChanelModal>; + title: 'views/sidebar/CreateChannelModal', + component: CreateChannelModal, +} satisfies Meta<typeof CreateChannelModal>;-export const Default: StoryFn<typeof CreateChanelModal> = (args) => <CreateChanelModal {...args} />; +export const Default: StoryFn<typeof CreateChannelModal> = (args) => <CreateChannelModal {...args} />;Also applies to: 7-9, 11-11
4-4: Split V1 and V2 stories into separate Storybook Meta filesDefaultVersion2 renders CreateChannelModalV2 while this file's Meta/component is CreateChanelModal — don’t mix different components under one Meta (it skews docs/args typing). Move the V2 story to its own stories file (or give it a separate Meta).
- Location: apps/meteor/client/sidebar/header/CreateChannel/CreateChannelModal.stories.tsx
- Quick removal (keep V2 in a separate file):
-import CreateChannelModalV2 from '../../../NavBarV2/NavBarPagesGroup/actions/CreateChannelModal'; ... -export const DefaultVersion2: StoryFn<typeof CreateChannelModalV2> = (args) => <CreateChannelModalV2 {...args} />;
- Example new file:
// apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/CreateChannelModal.stories.tsx import type { Meta, StoryFn } from '@storybook/react'; import CreateChannelModal from './CreateChannelModal'; export default { title: 'views/navbar-v2/CreateChannelModal', component: CreateChannelModal, } satisfies Meta<typeof CreateChannelModal>; export const Default: StoryFn<typeof CreateChannelModal> = (args) => <CreateChannelModal {...args} />;
- Also fix the "CreateChanel" typo in the title/import to "CreateChannel".
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/meteor/client/sidebar/header/CreateChannel/__snapshots__/CreateChannelModal.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/sidebar/header/CreateTeam/__snapshots__/CreateTeamModal.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (9)
apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/CreateChannelModal.tsx(1 hunks)apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/CreateTeamModal.tsx(4 hunks)apps/meteor/client/sidebar/header/CreateChannel/CreateChannelModal.spec.tsx(1 hunks)apps/meteor/client/sidebar/header/CreateChannel/CreateChannelModal.stories.tsx(1 hunks)apps/meteor/client/sidebar/header/CreateChannel/CreateChannelModal.tsx(1 hunks)apps/meteor/client/sidebar/header/CreateTeam/CreateTeamModal.spec.tsx(1 hunks)apps/meteor/client/sidebar/header/CreateTeam/CreateTeamModal.stories.tsx(1 hunks)apps/meteor/client/sidebar/header/CreateTeam/CreateTeamModal.tsx(4 hunks)apps/meteor/client/stories/contexts/RouterContextMock.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
apps/meteor/client/sidebar/header/CreateTeam/CreateTeamModal.spec.tsx (1)
packages/mock-providers/src/index.ts (1)
mockAppRoot(3-3)
apps/meteor/client/sidebar/header/CreateChannel/CreateChannelModal.spec.tsx (2)
apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/testCreateChannelModal.tsx (1)
testCreateChannelModal(9-179)packages/mock-providers/src/index.ts (1)
mockAppRoot(3-3)
apps/meteor/client/sidebar/header/CreateTeam/CreateTeamModal.stories.tsx (1)
apps/meteor/client/sidebar/header/CreateChannel/CreateChannelModal.stories.tsx (2)
Default(11-11)DefaultVersion2(12-12)
apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/CreateTeamModal.tsx (1)
apps/meteor/app/utils/lib/i18n.ts (1)
t(6-6)
apps/meteor/client/sidebar/header/CreateTeam/CreateTeamModal.tsx (1)
apps/meteor/app/utils/lib/i18n.ts (1)
t(6-6)
apps/meteor/client/sidebar/header/CreateChannel/CreateChannelModal.stories.tsx (1)
apps/meteor/client/sidebar/header/CreateTeam/CreateTeamModal.stories.tsx (2)
Default(11-11)DefaultVersion2(12-12)
⏰ 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). (7)
- GitHub Check: 🔨 Test Storybook / Test Storybook
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: 🔎 Code Check / Code Lint
- GitHub Check: 📦 Meteor Build - coverage
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (5)
apps/meteor/client/sidebar/header/CreateChannel/CreateChannelModal.tsx (1)
313-314: FieldHint swap for encrypted hint — LGTMCorrect component, stable id, and aria-describedby wiring are preserved.
apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/CreateChannelModal.tsx (1)
317-317: FieldHint swap for encrypted hint — LGTMConsistent with the rest of the modal; a11y refs intact.
apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/CreateTeamModal.tsx (1)
243-246: Replace FieldDescription → FieldHint across hints — LGTMPrivate, Encrypted, Read-only, and Broadcast hints now use FieldHint with correct ids and aria-describedby.
Also applies to: 272-272, 292-295, 313-314
apps/meteor/client/sidebar/header/CreateTeam/CreateTeamModal.tsx (1)
240-243: Replace FieldDescription → FieldHint across hints — LGTMAll four hints switched to FieldHint with stable ids and aria-describedby references.
Also applies to: 269-269, 289-292, 310-311
apps/meteor/client/sidebar/header/CreateChannel/CreateChannelModal.stories.tsx (1)
1-2: Good use ofsatisfiesfor Meta typing.Typing is precise and future-proof; no changes needed.
Also applies to: 9-9
MartinSchoeler
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Proposed changes (including videos or screenshots)
Enforce the
FieldHintusage in order to keep visual consistencybefore
after
Issue(s)
Steps to test or reproduce
Further comments
CORE-1345
Summary by CodeRabbit