Skip to content
Draft
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 103 additions & 13 deletions .maestro/tests/room/message-markdown-click.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,19 @@ name: Message Markdown
jsEngine: graaljs
onFlowStart:
- runFlow: '../../helpers/setup.yaml'
onFlowComplete:
- evalScript: ${output.utils.deleteCreatedUsers()}
tags:
- test-9

---
- evalScript: ${output.user = output.utils.createUser()}

- runFlow:
file: '../../helpers/login-with-deeplink.yaml'
env:
USERNAME: ${output.user.username}
PASSWORD: ${output.user.password}
USERNAME: ${output.account.adminUser}
PASSWORD: ${output.account.adminPassword}
- runFlow:
file: '../../helpers/search-and-navigate-room.yaml'
env:
ROOM: 'maestro-message-clickable-test'
ROOM: 'maestro-message-clickable-test-temp'

- extendedWaitUntil:
visible:
Expand Down Expand Up @@ -63,7 +59,7 @@ tags:
# Wait for room view to be visible again
- extendedWaitUntil:
visible:
id: 'room-view-title-maestro-message-clickable-test'
id: 'room-view-title-maestro-message-clickable-test-temp'
timeout: 60000

# Tap on the URL link to open alert
Expand All @@ -78,7 +74,7 @@ tags:
platform: iOS
commands:
- tapOn:
point: 66%,63%
point: 66%,59%

# Verify alert is shown with the link
- extendedWaitUntil:
Expand Down Expand Up @@ -106,7 +102,7 @@ tags:
platform: iOS
commands:
- longPressOn:
point: 66%,63%
point: 66%,59%

# Verify clipboard has the link alert
- extendedWaitUntil:
Expand Down Expand Up @@ -143,7 +139,7 @@ tags:
# Wait for room view to be visible again
- extendedWaitUntil:
visible:
id: 'room-view-title-maestro-message-clickable-test'
id: 'room-view-title-maestro-message-clickable-test-temp'
timeout: 60000

# Tap on message with thread to open thread room
Expand All @@ -163,7 +159,7 @@ tags:
# Wait for room view to be visible again
- extendedWaitUntil:
visible:
id: 'room-view-title-maestro-message-clickable-test'
id: 'room-view-title-maestro-message-clickable-test-temp'
timeout: 60000

# Tap on "View thread" button
Expand All @@ -183,7 +179,7 @@ tags:
# Wait for room view to be visible again
- extendedWaitUntil:
visible:
id: 'room-view-title-maestro-message-clickable-test'
id: 'room-view-title-maestro-message-clickable-test-temp'
timeout: 60000

# Now tap on a message within the thread
Expand All @@ -195,3 +191,97 @@ tags:
visible:
id: 'room-view-title-message with thread'
timeout: 60000

# Go back to main room
- tapOn:
id: header-back

# Wait for room view to be visible again
- extendedWaitUntil:
visible:
id: 'room-view-title-maestro-message-clickable-test-temp'
timeout: 60000

# Long press on avatar and make sure it show user info
- longPressOn: "rohit.bansal's avatar"
- extendedWaitUntil:
visible:
text: '.*User info*.'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Incorrect contains-pattern for text selectors.

These text selectors use '.*keyword*.' instead of the correct '.*keyword.*' form (leading and trailing .*). Since the target keyword sits at the end of the matched string in all three cases, the pattern will fail to match at runtime.

Based on learnings, in Maestro YAML selector fields, use the contains pattern '.*keyword.*' (leading and trailing .*); '.*keyword*.' is incorrect and will fail to match cases where the keyword appears at the end of the element's text.

🩹 Proposed fix
- text: '.*User info*.'
+ text: '.*User info.*'
- text: '.*Button tap received! This reply is private to you*.'
+ text: '.*Button tap received! This reply is private to you.*'
- text: '.*A very secret message visible after clicking Ephemeral Button*.'
+ text: '.*A very secret message visible after clicking Ephemeral Button.*'

Also applies to: 267-267, 286-286

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.maestro/tests/room/message-markdown-click.yaml at line 209, The text
selector patterns in the message markdown click Maestro YAML use the wrong
contains-match form (`.*keyword*.`), which won’t match when the keyword appears
at the end of the element text. Update the affected selector fields in this test
to use the proper contains pattern (`.*keyword.*`) consistently for the entries
matching “User info” and the other two similar cases in this file.

Source: Learnings

timeout: 60000

# Go back to main room
- tapOn: Back
- extendedWaitUntil:
visible:
id: 'room-view-title-maestro-message-clickable-test-temp'
timeout: 60000

# This is to make sure that button is in correct state before we test
- runFlow:
when:
visible:
id: 'thread-unfollow'
commands:
- tapOn:
id: 'thread-unfollow'

# Click follow to notification button and make sure it stays in room and the button changes to unfollow
- tapOn:
id: 'thread-follow'
- extendedWaitUntil:
visible:
id: 'thread-unfollow'
timeout: 10000
- extendedWaitUntil:
visible:
id: 'room-view-title-maestro-message-clickable-test-temp'
timeout: 60000

# Click unfollow to notification button and make sure it stays in room and the button changes to follow
- tapOn:
id: 'thread-unfollow'
- extendedWaitUntil:
visible:
id: 'thread-follow'
timeout: 10000
- extendedWaitUntil:
visible:
id: 'room-view-title-maestro-message-clickable-test-temp'
timeout: 60000

# Click UI Kit Button
- runFlow:
when:
platform: iOS
commands:
- tapOn:
point: '55%,86%'
- runFlow:
when:
platform: Android
commands:
- tapOn: Tap Me

- extendedWaitUntil:
visible:
text: '.*Button tap received! This reply is private to you*.'
timeout: 10000

# Click Emphemeral UI Kit Button
- tapOn:
id: message-composer-input
- inputText:
text: '/send-emphemeral-button'
- extendedWaitUntil:
visible:
id: autocomplete-item-send-emphemeral-button
timeout: 10000
- tapOn:
id: autocomplete-item-send-emphemeral-button
- tapOn:
id: 'message-composer-send'
- tapOn: 'Ephemeral Button'
- extendedWaitUntil:
visible:
text: '.*A very secret message visible after clicking Ephemeral Button*.'
timeout: 10000
6 changes: 5 additions & 1 deletion app/containers/ThreadDetails.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,10 @@ const ThreadDetails = ({ item, user, badgeColor, toggleFollowThread, style }: IT

<View style={styles.detailContainer}>
<CustomIcon name='user' size={24} />
<Text style={[styles.detailText, { color: themes[theme].fontSecondaryInfo }]} numberOfLines={1}>
<Text
testID={`thread-user-${replies}`}
style={[styles.detailText, { color: themes[theme].fontSecondaryInfo }]}
numberOfLines={1}>
{replies}
</Text>
</View>
Expand All @@ -88,6 +91,7 @@ const ThreadDetails = ({ item, user, badgeColor, toggleFollowThread, style }: IT
{badgeColor ? <View style={[styles.badge, { backgroundColor: badgeColor }]} /> : null}
<Touch
accessibilityLabel={i18n.t(isFollowing ? 'Unfollow_thread' : 'Follow_thread')}
testID={isFollowing ? 'thread-unfollow' : 'thread-follow'}
onPress={() => toggleFollowThread?.(isFollowing, item.id)}>
<CustomIcon size={24} name={isFollowing ? 'notification' : 'notification-disabled'} />
</Touch>
Expand Down
63 changes: 36 additions & 27 deletions app/containers/Touch.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
import { forwardRef, type ReactNode } from 'react';
import { RectButton, type RectButtonProps } from 'react-native-gesture-handler';
import { forwardRef, type ReactNode, type RefObject } from 'react';
import {
View,
StyleSheet,
type ViewStyle,
type StyleProp,
type AccessibilityActionEvent,
type AccessibilityActionInfo
type AccessibilityActionInfo,
TouchableOpacity,
TouchableHighlight,
type TouchableWithoutFeedbackProps
} from 'react-native';
import { withKeyboardFocus } from 'react-native-external-keyboard';

import { useTheme } from '../theme';
import { isIOS } from '../lib/methods/helpers';

export interface ITouchProps extends RectButtonProps {
export interface ITouchProps extends TouchableWithoutFeedbackProps {
children: ReactNode;
accessible?: boolean;
accessibilityLabel?: string;
Expand All @@ -21,25 +24,29 @@ export interface ITouchProps extends RectButtonProps {
onAccessibilityAction?: (event: AccessibilityActionEvent) => void;
testID?: string;
rectButtonStyle?: StyleProp<ViewStyle>;
disabled?: boolean;
enabled?: boolean;
android_rippleColor?: string;
componentRef?: RefObject<View | null>;
}

const KeyboardRectButton = withKeyboardFocus(RectButton);
const Component = isIOS ? TouchableOpacity : TouchableHighlight;
const KeyboardComponent = withKeyboardFocus(Component);

const Touch = forwardRef<any, ITouchProps>(
const Touch = forwardRef<View, ITouchProps>(
(
{
children,
onPress,
underlayColor,
android_rippleColor,
accessible,
accessibilityLabel,
accessibilityHint,
accessibilityActions,
onAccessibilityAction,
style,
rectButtonStyle,
disabled,
enabled = true,
componentRef,
...props
},
ref
Expand Down Expand Up @@ -75,29 +82,31 @@ const Touch = forwardRef<any, ITouchProps>(
marginStart,
marginTop
};
const touchableProps = isIOS ? {} : { underlayColor: android_rippleColor ?? colors.surfaceNeutral, activeOpacity: 1 };

return (
<KeyboardRectButton
<KeyboardComponent
ref={ref}
// Library types componentRef as RefObject<View>, but useRef<View>(null) yields RefObject<View | null>. The lib only reads .current with a null check, so the cast is safe.
componentRef={componentRef as RefObject<View>}
onPress={onPress}
activeOpacity={1}
underlayColor={underlayColor || colors.surfaceNeutral}
rippleColor={colors.surfaceNeutral}
focusable={!disabled}
canBeFocused={!disabled}
accessible={accessible}
accessibilityRole={props.accessibilityRole}
accessibilityLabel={accessibilityLabel}
accessibilityHint={accessibilityHint}
accessibilityActions={accessibilityActions}
onAccessibilityAction={onAccessibilityAction}
style={[rectButtonStyle, marginStyles, { backgroundColor, borderRadius }]}
{...touchableProps}
{...props}
enabled={!disabled}>
<View
accessible={accessible}
accessibilityRole={props.accessibilityRole}
accessibilityLabel={accessibilityLabel}
accessibilityHint={accessibilityHint}
accessibilityActions={accessibilityActions}
onAccessibilityAction={onAccessibilityAction}
style={viewStyle}>
{children}
</View>
</KeyboardRectButton>
disabled={!enabled}
focusable={enabled}
canBeFocused={enabled}>
{/* The accessibility props live on the focusable Touchable above. The inner View is a
layout-only container; marking it accessible would create a second sibling node with
the same label, causing double VoiceOver announcements and confusing TalkBack swipe nav. */}
<View style={viewStyle}>{children}</View>
</KeyboardComponent>
);
}
);
Expand Down
2 changes: 1 addition & 1 deletion app/containers/message/Message.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { WidthAwareView } from './Components/WidthAwareView';
import MessageTime from './Time';
import { useResponsiveLayout } from '../../lib/hooks/useResponsiveLayout/useResponsiveLayout';
import Quote from './Components/Attachments/Quote';
import Touch from './Touch';
import Touch from '../Touch';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== File map ==\n'
git ls-files app/containers/message/Message.tsx app/containers/Touch.tsx app/containers/message | sed 's#^`#-` #'

printf '\n== Relevant outlines ==\n'
ast-grep outline app/containers/message/Message.tsx --view expanded || true
printf '\n---\n'
ast-grep outline app/containers/Touch.tsx --view expanded || true

printf '\n== Targeted reads ==\n'
sed -n '1,120p' app/containers/Touch.tsx
printf '\n--- Message.tsx around import and Touch usage ---\n'
sed -n '1,260p' app/containers/message/Message.tsx

printf '\n== Search for enabled/disabled usage with Touch ==\n'
rg -n "enabled=|disabled=|<Touch|Touch " app/containers/message app/containers/Touch.tsx

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 50390


Use enabled={!isDisabled} here

Touch ignores disabled now, so this prop no longer disables the message press target. Info, archived, temp, and jitsi_call_started messages stay tappable.

Suggested fix
-					disabled={isDisabled}
+					enabled={!isDisabled}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import Touch from '../Touch';
enabled={!isDisabled}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/containers/message/Message.tsx` at line 26, The message press target is
still using the old disabled prop on Touch, but Touch now ignores it, so the
Message component needs to gate interaction with enabled instead. Update the
touch wrapper in Message.tsx to pass enabled={!isDisabled} and use the Message
component’s existing disabled-state logic (for info, archived, temp, and
jitsi_call_started messages) to determine whether the target should be tappable.

import { useLastFocusedMessageRef } from '../../lib/a11y/useLastFocusedMessageRef';
import { useMessageAccessibilityLabel } from './hooks/useMessageAccessibilityLabel';
import { useMessageAccessibilityActions } from './hooks/useMessageAccessibilityActions';
Expand Down
Loading
Loading