-
Notifications
You must be signed in to change notification settings - Fork 0
RC-T40 Chat Fixes #41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| @import 'tailwindcss/theme.css' layer(theme); | ||
| @import 'tailwindcss/preflight.css' layer(base); | ||
| @import 'tailwindcss/utilities.css'; | ||
| @import 'nativewind/theme'; | ||
|
|
||
| @import './theme-tokens.css'; | ||
|
|
||
| /* Web dark mode: the .dark class GluestackUIProvider puts on <html> (see index.web.tsx). It is | ||
| always present — explicit modes set .dark/.light directly, and system mode sets one from the | ||
| media query — so the class alone is authoritative. Matching prefers-color-scheme here as well | ||
| would make an explicit Light choice render dark utilities on a dark-themed OS. */ | ||
| @custom-variant dark (&:where(.dark, .dark *)); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,5 @@ | ||
| { | ||
| "tailwind": { | ||
| "config": "tailwind.config.js", | ||
| "css": "global.css" | ||
| }, | ||
| "app": { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,7 +3,7 @@ import { type TFunction } from 'i18next'; | |||||
|
|
||||||
| import { ChatChannelType, type ChatChannelResultData } from '@/models/v4/chat'; | ||||||
|
|
||||||
| import { copyToClipboard, getChannelDisplayName, getImageMimeType, hasLink, linkifySegments } from '../chat-utils'; | ||||||
| import { copyToClipboard, getChannelDisplayName, getImageMimeType, groupChannels, hasLink, linkifySegments } from '../chat-utils'; | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Use the configured path alias for Line 6 uses Suggested import-import { copyToClipboard, getChannelDisplayName, getImageMimeType, groupChannels, hasLink, linkifySegments } from '../chat-utils';
+import { copyToClipboard, getChannelDisplayName, getImageMimeType, groupChannels, hasLink, linkifySegments } from '`@/components/chat/chat-utils`';As per coding guidelines, 📝 Committable suggestion
Suggested change
🤖 Prompt for AI AgentsSource: Coding guidelines |
||||||
|
|
||||||
| jest.mock('expo-clipboard', () => ({ setStringAsync: jest.fn() })); | ||||||
|
|
||||||
|
|
@@ -34,6 +34,31 @@ describe('chat-utils', () => { | |||||
| }); | ||||||
| }); | ||||||
|
|
||||||
| describe('groupChannels', () => { | ||||||
| it('buckets every incident-scoped channel type into the incidents section', () => { | ||||||
| const grouped = groupChannels([ | ||||||
| buildChannel({ ChatChannelId: 'a', ChannelType: ChatChannelType.Incident }), | ||||||
| buildChannel({ ChatChannelId: 'b', ChannelType: ChatChannelType.IncidentLane }), | ||||||
| buildChannel({ ChatChannelId: 'c', ChannelType: ChatChannelType.IncidentCommand }), | ||||||
| buildChannel({ ChatChannelId: 'd', ChannelType: ChatChannelType.IncidentLeads }), | ||||||
| buildChannel({ ChatChannelId: 'e', ChannelType: ChatChannelType.IncidentDispatch }), | ||||||
| ]); | ||||||
| expect(grouped.incidents.map((c) => c.ChatChannelId).sort()).toEqual(['a', 'b', 'c', 'd', 'e']); | ||||||
| expect(grouped.channels).toHaveLength(0); | ||||||
| }); | ||||||
|
|
||||||
| it('buckets the unit dispatch line into the channels section', () => { | ||||||
| const grouped = groupChannels([buildChannel({ ChatChannelId: 'ud', ChannelType: ChatChannelType.UnitDispatch })]); | ||||||
| expect(grouped.channels.map((c) => c.ChatChannelId)).toEqual(['ud']); | ||||||
| expect(grouped.incidents).toHaveLength(0); | ||||||
| }); | ||||||
|
|
||||||
| it('skips archived channels', () => { | ||||||
| const grouped = groupChannels([buildChannel({ ChatChannelId: 'x', ChannelType: ChatChannelType.Incident, IsArchived: true })]); | ||||||
| expect(grouped.incidents).toHaveLength(0); | ||||||
| }); | ||||||
| }); | ||||||
|
|
||||||
| describe('getImageMimeType', () => { | ||||||
| it('prefers the picker asset mimeType when available', () => { | ||||||
| expect(getImageMimeType('file:///photos/photo.jpg', 'image/png')).toBe('image/png'); | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import React from 'react'; | ||
| import { Platform } from 'react-native'; | ||
|
|
||
| /** | ||
| * Android field metrics, applied from JS because the class layer cannot express them correctly. | ||
| * | ||
| * Measured on device, each case a real Input/InputField: | ||
| * - the class `h-full` (height: 100%) resolves taller than the fixed-height parent on Android, and | ||
| * the parent's overflow-hidden then clips the top of the glyphs; | ||
| * - an explicit pixel height matching the parent renders correctly; | ||
| * - overriding only the lineHeight, at either the class or the style layer, does not help; | ||
| * - lineHeight 0 (what iOS uses) hides Android text completely, and a later `undefined` does not | ||
| * clear the value the size class sets. | ||
| * | ||
| * So Android gets a concrete height plus a lineHeight near the font size, and drops the extra font | ||
| * padding. iOS keeps the zero lineHeight that upstream applied through `ios:leading-[0px]`; that class | ||
| * is gone from the base style so the value can be chosen per platform here. | ||
| */ | ||
| const ANDROID_FIELD_METRICS: Record<string, { height: number; lineHeight: number }> = { | ||
| sm: { height: 36, lineHeight: 18 }, | ||
| md: { height: 40, lineHeight: 20 }, | ||
| lg: { height: 44, lineHeight: 22 }, | ||
| xl: { height: 48, lineHeight: 25 }, | ||
| }; | ||
|
|
||
| export const useTextFieldVerticalFix = (size: string | undefined) => | ||
| React.useMemo(() => { | ||
| if (Platform.OS === 'ios') { | ||
| return { lineHeight: 0 } as const; | ||
| } | ||
| if (Platform.OS === 'android') { | ||
| const metrics = ANDROID_FIELD_METRICS[size ?? 'md'] ?? ANDROID_FIELD_METRICS.md; | ||
| return { height: metrics.height, lineHeight: metrics.lineHeight, includeFontPadding: false, textAlignVertical: 'center' } as const; | ||
| } | ||
| return undefined; | ||
| }, [size]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Bind the API result to the current
channelIdbefore enabling the composer.apiChannelis not cleared or associated with the route that produced it. If this screen remains mounted whilechannelIdchanges from a resolved deep-linked channel to another channel that is absent from local state,channelcontinues to use the old channel while the new request is pending. If the request fails, the old channel remains indefinitely.The composer can then send the new
channelIdwith the old channel'sisCommandChannelandisFrozenvalues. Store the resolvedchannelIdwith the API result, use it only when it matches the current route, clear it while resolving, and abort the request during cleanup. The suppliedsrc/api/chat/chat.tscontract accepts an optionalAbortSignal.Add a regression test that changes
channelIdafter one channel resolves and verifies that the composer remains disabled until the new channel resolves. As per coding guidelines:generate tests for new components, services, and logic.Proposed route-scoped resolution
Also applies to: 181-181
🤖 Prompt for AI Agents
Source: Coding guidelines