Skip to content
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

17047 migrate PressableWithSecondaryInteraction to PressableWithFeedback component #20251

Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,13 @@ const BaseAnchorForCommentsOnly = (props) => {
onPress={linkProps.onPress}
onPressIn={props.onPressIn}
onPressOut={props.onPressOut}
accessibilityRole="link"
robertKozik marked this conversation as resolved.
Show resolved Hide resolved
accessibilityLabel={props.href}
>
<Tooltip text={props.href}>
<Text
ref={(el) => (linkRef = el)}
style={StyleSheet.flatten([props.style, defaultTextStyle])}
accessibilityRole="link"
hrefAttrs={{
rel: props.rel,
target: isEmail ? '_self' : props.target,
Expand Down
4 changes: 3 additions & 1 deletion src/components/LHNOptionsList/OptionRowLHN.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,12 @@ const OptionRowLHN = (props) => {
props.isFocused ? styles.sidebarLinkActive : null,
hovered && !props.isFocused ? props.hoverStyle : null,
]}
accessibilityRole="button"
robertKozik marked this conversation as resolved.
Show resolved Hide resolved
accessibilityLabel={props.translate('accessibilityHints.navigatesToChat')}
>
<View
accessibilityHint={props.translate('accessibilityHints.navigatesToChat')}
style={sidebarInnerRowStyle}
accessibilityHint={props.translate('accessibilityHints.navigatesToChat')}
>
<View style={[styles.flexRow, styles.alignItemsCenter]}>
{!_.isEmpty(optionItem.icons) &&
Expand Down
2 changes: 2 additions & 0 deletions src/components/MenuItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ const MenuItem = (props) => {
]}
disabled={props.disabled}
ref={props.forwardedRef}
accessibilityRole="menuitem"
robertKozik marked this conversation as resolved.
Show resolved Hide resolved
accessibilityLabel={props.title}
Copy link
Contributor

@abdulrahuman5196 abdulrahuman5196 Sep 4, 2023

Choose a reason for hiding this comment

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

props.title was of number 0 in some cases which caused app crash as mentioned here #25451 (comment).
We should make sure the values we pass are string.

>
{({hovered, pressed}) => (
<>
Expand Down
8 changes: 4 additions & 4 deletions src/components/PressableWithSecondaryInteraction/index.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import _ from 'underscore';
import React, {Component} from 'react';
import {Pressable} from 'react-native';
import * as pressableWithSecondaryInteractionPropTypes from './pressableWithSecondaryInteractionPropTypes';
import styles from '../../styles/styles';
import * as DeviceCapabilities from '../../libs/DeviceCapabilities';
import * as StyleUtils from '../../styles/StyleUtils';
import PressableWithFeedback from '../Pressable/PressableWithFeedback';

/**
* This is a special Pressable that calls onSecondaryInteraction when LongPressed, or right-clicked.
Expand Down Expand Up @@ -76,19 +76,19 @@ class PressableWithSecondaryInteraction extends Component {

// On Web, Text does not support LongPress events thus manage inline mode with styling instead of using Text.
return (
<Pressable
<PressableWithFeedback
style={StyleUtils.combineStyles(this.props.inline ? styles.dInline : this.props.style)}
onPressIn={this.props.onPressIn}
onLongPress={this.props.onSecondaryInteraction ? this.executeSecondaryInteraction : undefined}
activeOpacity={this.props.activeOpacity}
pressDimmingValue={this.props.activeOpacity}
onPressOut={this.props.onPressOut}
onPress={this.props.onPress}
ref={(el) => (this.pressableRef = el)}
// eslint-disable-next-line react/jsx-props-no-spreading
{...defaultPressableProps}
>
{this.props.children}
</Pressable>
</PressableWithFeedback>
);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import _ from 'underscore';
import React, {forwardRef} from 'react';
import {Pressable} from 'react-native';
import * as pressableWithSecondaryInteractionPropTypes from './pressableWithSecondaryInteractionPropTypes';
import Text from '../Text';
import HapticFeedback from '../../libs/HapticFeedback';
import PressableWithFeedback from '../Pressable/PressableWithFeedback';

/**
* This is a special Pressable that calls onSecondaryInteraction when LongPressed.
Expand All @@ -13,22 +12,20 @@ import HapticFeedback from '../../libs/HapticFeedback';
*/
const PressableWithSecondaryInteraction = (props) => {
// Use Text node for inline mode to prevent content overflow.
const Node = props.inline ? Text : Pressable;
const Node = props.inline ? Text : PressableWithFeedback;
const executeSecondaryInteraction = (e) => {
e.preventDefault();
props.onSecondaryInteraction(e);
};

return (
<Node
ref={props.forwardedRef}
onPress={props.onPress}
onLongPress={(e) => {
if (!props.onSecondaryInteraction) {
return;
}
e.preventDefault();
HapticFeedback.longPress();
props.onSecondaryInteraction(e);
}}
onLongPress={props.onSecondaryInteraction ? executeSecondaryInteraction : undefined}
onPressIn={props.onPressIn}
onPressOut={props.onPressOut}
activeOpacity={props.activeOpacity}
pressDimmingValue={props.activeOpacity}
// eslint-disable-next-line react/jsx-props-no-spreading
{..._.omit(props, 'onLongPress')}
>
Expand Down
2 changes: 2 additions & 0 deletions src/components/Reactions/EmojiReactionBubble.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ const EmojiReactionBubble = (props) => (
enableLongPressWithHover={props.isSmallScreenWidth}
// Prevent text input blur when emoji reaction is clicked
onMouseDown={(e) => e.preventDefault()}
accessibilityRole="button"
Copy link
Contributor

Choose a reason for hiding this comment

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

NAB since this already exists in many places.

Suggested change
accessibilityRole="button"
accessibilityRole={CONST.ACCESSIBILITY_ROLE.BUTTON}

@luacmartins after this PR merged, let's create follow-up issue for replacing all hardcoded roles

accessibilityLabel={props.emojiCodes.join('')}
>
<Text style={[styles.emojiReactionBubbleText, styles.userSelectNone, StyleUtils.getEmojiReactionBubbleTextStyle(props.isContextMenu)]}>{props.emojiCodes.join('')}</Text>
{props.count > 0 && <Text style={[styles.reactionCounterText, styles.userSelectNone, StyleUtils.getEmojiReactionCounterTextStyle(props.hasUserReacted)]}>{props.count}</Text>}
Expand Down
2 changes: 1 addition & 1 deletion src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ function canDeleteReportAction(reportAction, reportID) {
return true;
}
const report = lodashGet(allReports, `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {});
const policy = lodashGet(allPolicies, `${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`, {});
const policy = lodashGet(allPolicies, `${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`) || {};
return policy.role === CONST.POLICY.ROLE.ADMIN;
}

Expand Down