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.

Sign up for GitHub

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

Jump to bottom

17046 — migrate PressableWithotFocus to GenericPressable #20202

Merged
merged 18 commits into from
Jun 16, 2023
Merged

17046 — migrate PressableWithotFocus to GenericPressable #20202

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
f42e37e
migrate PressableWithotFocus to GenericPressable
robertKozik Jun 5, 2023
924e96a
hoist GenericPressable proptypes to PressableWithoutFocus
robertKozik Jun 6, 2023
c1d7329
move PressableWithoutFoucs to Pressable folder
robertKozik Jun 7, 2023
bcea8ea
fix propTypes of PressableWithFeedback - add missing propTypes
robertKozik Jun 7, 2023
0f9bc39
hoist propTypes of ImageRenderer to the top of the file as stated in …
robertKozik Jun 7, 2023
af0e8c4
Remove wrapperStyle proptype from PressableWithFeedback.js
robertKozik Jun 12, 2023
1f4d4b9
rename styling prop of PressableWithoutFocus to 'style'
robertKozik Jun 12, 2023
a6333e5
fix lint
robertKozik Jun 12, 2023
fa9ca8b
Merge branch 'main' into 17046-migrate-PressableWithoutFocus
robertKozik Jun 12, 2023
6e47178
change import of PressableWithoutFocus in ProfilePage.js
robertKozik Jun 12, 2023
6bfc85a
add accessibilityLabel for ProfilePage, pass styles as arrays
robertKozik Jun 12, 2023
65afa46
Merge branch 'main' into 17046-migrate-PressableWithoutFocus
robertKozik Jun 14, 2023
ebce2d6
Change accessibility role of ImageRenderer pressable to imagebutton
robertKozik Jun 14, 2023
9b167ac
fix omitted prop name in PressableWithoutFocus
robertKozik Jun 14, 2023
10e3774
change accessibilityRole from button to imagebutton
robertKozik Jun 14, 2023
8734e0b
change accessibility label for ImageRenderer component
robertKozik Jun 15, 2023
afb9b28
fix typo in translations
robertKozik Jun 15, 2023
30e0a94
Merge branch 'main' into 17046-migrate-PressableWithoutFocus
robertKozik Jun 16, 2023
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
13 changes: 9 additions & 4 deletions src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@ import htmlRendererPropTypes from './htmlRendererPropTypes';
import AttachmentModal from '../../AttachmentModal';
import styles from '../../../styles/styles';
import ThumbnailImage from '../../ThumbnailImage';
import PressableWithoutFocus from '../../PressableWithoutFocus';
import PressableWithoutFocus from '../../Pressable/PressableWithoutFocus';
import CONST from '../../../CONST';
import {ShowContextMenuContext, showContextMenuForReport} from '../../ShowContextMenuContext';
import tryResolveUrlFromApiRoot from '../../../libs/tryResolveUrlFromApiRoot';
import * as ReportUtils from '../../../libs/ReportUtils';
import withLocalize, {withLocalizePropTypes} from '../../withLocalize';

const propTypes = {...htmlRendererPropTypes, ...withLocalizePropTypes};

function ImageRenderer(props) {
const htmlAttribs = props.tnode.attributes;
Expand Down Expand Up @@ -60,9 +63,11 @@ function ImageRenderer(props) {
>
{({show}) => (
<PressableWithoutFocus
style={styles.noOutline}
style={[styles.noOutline]}
onPress={show}
onLongPress={(event) => showContextMenuForReport(event, anchor, report.reportID, action, checkIfContextMenuActive, ReportUtils.isArchivedRoom(report))}
accessibilityRole="imagebutton"
accessibilityLabel={props.translate('accessibilityHints.viewAttachment')}
>
<ThumbnailImage
previewSourceURL={previewSource}
Expand All @@ -79,7 +84,7 @@ function ImageRenderer(props) {
);
}

ImageRenderer.propTypes = htmlRendererPropTypes;
ImageRenderer.propTypes = propTypes;
ImageRenderer.displayName = 'ImageRenderer';

export default ImageRenderer;
export default withLocalize(ImageRenderer);
4 changes: 2 additions & 2 deletions src/components/Pressable/PressableWithFeedback.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import * as StyleUtils from '../../styles/StyleUtils';
const omittedProps = ['style', 'pressStyle', 'hoverStyle', 'focusStyle', 'wrapperStyle'];

const PressableWithFeedbackPropTypes = {
..._.omit(GenericPressablePropTypes.pressablePropTypes, omittedProps),
...GenericPressablePropTypes.pressablePropTypes,
/**
* Determines what opacity value should be applied to the underlaying view when Pressable is pressed.
* To disable dimming, pass 1 as pressDimmingValue
Expand All @@ -31,7 +31,7 @@ const PressableWithFeedbackPropTypes = {
};

const PressableWithFeedbackDefaultProps = {
..._.omit(GenericPressablePropTypes.defaultProps, omittedProps),
...GenericPressablePropTypes.defaultProps,
pressDimmingValue: variables.pressDimValue,
hoverDimmingValue: variables.hoverDimValue,
nativeID: '',
Expand Down
20 changes: 14 additions & 6 deletions src/components/PressableWithoutFocus.js → ...onents/Pressable/PressableWithoutFocus.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import React from 'react';
import {Pressable} from 'react-native';
import _ from 'underscore';
import PropTypes from 'prop-types';
import GenericPressable from './GenericPressable';
import genericPressablePropTypes from './GenericPressable/PropTypes';

const propTypes = {
/** Element that should be clickable */
Expand All @@ -14,11 +16,14 @@ const propTypes = {

/** Styles that should be passed to touchable container */
// eslint-disable-next-line react/forbid-prop-types
styles: PropTypes.arrayOf(PropTypes.object),
style: PropTypes.arrayOf(PropTypes.object),

/** Proptypes of pressable component used for implementation */
...genericPressablePropTypes.pressablePropTypes,
};

const defaultProps = {
styles: [],
style: [],
onLongPress: undefined,
};

Expand All @@ -41,15 +46,18 @@ class PressableWithoutFocus extends React.Component {
}

render() {
const restProps = _.omit(this.props, ['children', 'onPress', 'onLongPress', 'style']);
return (
<Pressable
<GenericPressable
onPress={this.pressAndBlur}
onLongPress={this.props.onLongPress}
ref={(el) => (this.pressableRef = el)}
style={this.props.styles}
style={this.props.style}
// eslint-disable-next-line react/jsx-props-no-spreading
{...restProps}
>
{this.props.children}
</Pressable>
</GenericPressable>
);
}
}
Expand Down
1 change: 1 addition & 0 deletions src/languages/en.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1397,6 +1397,7 @@ export default {
chatUserDisplayNames: 'Chat user display names',
scrollToNewestMessages: 'Scroll to newest messages',
prestyledText: 'Prestyled text',
viewAttachment: 'View attachment',
},
parentReportAction: {
deletedMessage: '[Deleted message]',
Expand Down
1 change: 1 addition & 0 deletions src/languages/es.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1863,6 +1863,7 @@ export default {
chatUserDisplayNames: 'Nombres de los usuarios del chat',
scrollToNewestMessages: 'Desplázate a los mensajes más recientes',
prestyledText: 'texto preestilizado',
viewAttachment: 'Ver archivo adjunto',
},
parentReportAction: {
deletedMessage: '[Mensaje eliminado]',
Expand Down
6 changes: 4 additions & 2 deletions src/pages/DetailsPage.js
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry If I am wrong, but It seems like we forgot to pass style prop in correct format here and it is causing warnings #21556.

<PressableWithoutFocus
    style={[styles.noOutline]}

Copy link
Member

Choose a reason for hiding this comment

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

yes, it seems so. That was original out of scope of this PR but we still renamed it as it was a small change but it seems this file got missed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks Rajat! @robertKozik If you agree, could you please make a follow up PR to fix #21556

Copy link
Contributor

Choose a reason for hiding this comment

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

@robertKozik 👋

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I missed the notifications about this one. I'll make the follow up to change the prop type of style ASAP

Copy link
Member

@parasharrajat parasharrajat Jul 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Feel free to tag me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@parasharrajat I've created the followup, but the but itself was already patched on main. I've unified the style type though. Let me know if it's worth a follow-up PR #22539

Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import * as ReportUtils from '../libs/ReportUtils';
import * as Expensicons from '../components/Icon/Expensicons';
import MenuItem from '../components/MenuItem';
import AttachmentModal from '../components/AttachmentModal';
import PressableWithoutFocus from '../components/PressableWithoutFocus';
import PressableWithoutFocus from '../components/Pressable/PressableWithoutFocus';
import * as Report from '../libs/actions/Report';
import OfflineWithFeedback from '../components/OfflineWithFeedback';
import AutoUpdateTime from '../components/AutoUpdateTime';
Expand Down Expand Up @@ -137,8 +137,10 @@ class DetailsPage extends React.PureComponent {
>
{({show}) => (
<PressableWithoutFocus
style={styles.noOutline}
style={[styles.noOutline]}
onPress={show}
accessibilityRole="imagebutton"
accessibilityLabel={this.props.translate('common.profile')}
>
<OfflineWithFeedback pendingAction={lodashGet(details, 'pendingFields.avatar', null)}>
<Avatar
Expand Down
6 changes: 4 additions & 2 deletions src/pages/ProfilePage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import * as ReportUtils from '../libs/ReportUtils';
import * as Expensicons from '../components/Icon/Expensicons';
import MenuItem from '../components/MenuItem';
import AttachmentModal from '../components/AttachmentModal';
import PressableWithoutFocus from '../components/PressableWithoutFocus';
import PressableWithoutFocus from '../components/Pressable/PressableWithoutFocus';
robertKozik marked this conversation as resolved.
Show resolved Hide resolved
import * as Report from '../libs/actions/Report';
import OfflineWithFeedback from '../components/OfflineWithFeedback';
import AutoUpdateTime from '../components/AutoUpdateTime';
Expand Down Expand Up @@ -148,8 +148,10 @@ function ProfilePage(props) {
>
{({show}) => (
<PressableWithoutFocus
style={styles.noOutline}
style={[styles.noOutline]}
onPress={show}
accessibilityLabel={props.translate('common.profile')}
accessibilityRole="imagebutton"
>
<OfflineWithFeedback pendingAction={lodashGet(details, 'pendingFields.avatar', null)}>
<Avatar
Expand Down