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

Download icon isn't turning green when clicking to download #9266

Merged
merged 12 commits into from
Jun 14, 2022
Merged

Download icon isn't turning green when clicking to download #9266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
665d66c
create HOC for button state complete
mollfpr Jun 1, 2022
227afbd
updating the component to use the HOC and remove unused code
mollfpr Jun 1, 2022
ff213e3
use HOC, updating the component to use class based, replace Touchable…
mollfpr Jun 1, 2022
7fbf845
Update src/components/withButtonStateComplete.js
mollfpr Jun 3, 2022
8c5af69
renaming
mollfpr Jun 3, 2022
992d302
updating to use new name of HOC
mollfpr Jun 3, 2022
356db33
renaming the state and the toggle function
mollfpr Jun 4, 2022
66fcab9
updating the state and the toggle function
mollfpr Jun 4, 2022
4d3f6aa
Merge branch 'main' of https://github.com/mollfpr/App into f-9124
mollfpr Jun 13, 2022
712e85a
auto reset on HeaderWithCloseButton
mollfpr Jun 13, 2022
b1844d3
minor jsdoc
mollfpr Jun 14, 2022
20107e2
add jsdoc more clearly
mollfpr Jun 14, 2022
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
38 changes: 13 additions & 25 deletions src/components/ContextMenuItem.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import Icon from './Icon';
import styles from '../styles/styles';
import * as StyleUtils from '../styles/StyleUtils';
import getButtonState from '../libs/getButtonState';
import withDelayToggleButtonState, {withDelayToggleButtonStatePropTypes} from './withDelayToggleButtonState';

const propTypes = {
/** Icon Component */
Expand All @@ -32,6 +33,8 @@ const propTypes = {

/** A description text to show under the title */
description: PropTypes.string,

...withDelayToggleButtonStatePropTypes,
};

const defaultProps = {
Expand All @@ -45,44 +48,29 @@ const defaultProps = {
class ContextMenuItem extends Component {
constructor(props) {
super(props);
this.state = {
success: false,
};
this.triggerPressAndUpdateSuccess = this.triggerPressAndUpdateSuccess.bind(this);
}

componentWillUnmount() {
if (!this.successResetTimer) {
return;
}

clearTimeout(this.successResetTimer);
this.triggerPressAndUpdateSuccess = this.triggerPressAndUpdateSuccess.bind(this);
}

/**
* Called on button press and mark the run
* Method to call parent onPress and toggleDelayButtonState
*/
triggerPressAndUpdateSuccess() {
Copy link
Contributor

Choose a reason for hiding this comment

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

missing JS doc

if (this.state.success) {
if (this.props.isDelayButtonStateComplete) {
return;
}
this.props.onPress();

// We only set the success state when we have icon or text to represent the success state
// We may want to replace this check by checking the Result from OnPress Callback in future.
if (this.props.successIcon || this.props.successText) {
this.setState({
success: true,
});
if (this.props.autoReset) {
this.successResetTimer = setTimeout(() => this.setState({success: false}), 1800);
}
this.props.toggleDelayButtonState(this.props.autoReset);
Copy link
Member

Choose a reason for hiding this comment

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

In context menu, we are never using the delay. So we should just use a state instead of the HOC.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I’ll apply the patch and try to take a look at the rest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rushatgabhane I just check that DetailsPage -> CommunicationsLink is using ContextMenuItem with props autoReset. Also in your patch, the HOC is deleted but HeaderWithCloseButton is still using the HOC.

Copy link
Member

Choose a reason for hiding this comment

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

Cool, yeah I missed CommunicationsLink.

}
}

render() {
const icon = this.state.success ? this.props.successIcon || this.props.icon : this.props.icon;
const text = this.state.success ? this.props.successText || this.props.text : this.props.text;
const icon = this.props.isDelayButtonStateComplete ? this.props.successIcon || this.props.icon : this.props.icon;
const text = this.props.isDelayButtonStateComplete ? this.props.successText || this.props.text : this.props.text;
return (
this.props.isMini
? (
Expand All @@ -94,14 +82,14 @@ class ContextMenuItem extends Component {
style={
({hovered, pressed}) => [
styles.reportActionContextMenuMiniButton,
StyleUtils.getButtonBackgroundColorStyle(getButtonState(hovered, pressed, this.state.success)),
StyleUtils.getButtonBackgroundColorStyle(getButtonState(hovered, pressed, this.props.isDelayButtonStateComplete)),
]
}
>
{({hovered, pressed}) => (
<Icon
src={icon}
fill={StyleUtils.getIconFillColor(getButtonState(hovered, pressed, this.state.success))}
fill={StyleUtils.getIconFillColor(getButtonState(hovered, pressed, this.props.isDelayButtonStateComplete))}
/>
)}
</Pressable>
Expand All @@ -112,7 +100,7 @@ class ContextMenuItem extends Component {
icon={icon}
onPress={this.triggerPressAndUpdateSuccess}
wrapperStyle={styles.pr9}
success={this.state.success}
success={this.props.isDelayButtonStateComplete}
description={this.props.description}
/>
)
Expand All @@ -123,4 +111,4 @@ class ContextMenuItem extends Component {
ContextMenuItem.propTypes = propTypes;
ContextMenuItem.defaultProps = defaultProps;

export default ContextMenuItem;
export default withDelayToggleButtonState(ContextMenuItem);
207 changes: 121 additions & 86 deletions src/components/HeaderWithCloseButton.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import React, {Component} from 'react';
import PropTypes from 'prop-types';
import {
View, TouchableOpacity, Keyboard,
View, Keyboard, Pressable,
} from 'react-native';
import styles from '../styles/styles';
import Header from './Header';
Expand All @@ -13,6 +13,10 @@ import withLocalize, {withLocalizePropTypes} from './withLocalize';
import Tooltip from './Tooltip';
import ThreeDotsMenu, {ThreeDotsMenuItemPropTypes} from './ThreeDotsMenu';
import VirtualKeyboard from '../libs/VirtualKeyboard';
import getButtonState from '../libs/getButtonState';
import * as StyleUtils from '../styles/StyleUtils';
import withDelayToggleButtonState, {withDelayToggleButtonStatePropTypes} from './withDelayToggleButtonState';
import compose from '../libs/compose';

const propTypes = {
/** Title of the Header */
Expand Down Expand Up @@ -75,6 +79,8 @@ const propTypes = {
}),

...withLocalizePropTypes,

...withDelayToggleButtonStatePropTypes,
};

const defaultProps = {
Expand All @@ -100,94 +106,123 @@ const defaultProps = {
},
};

const HeaderWithCloseButton = props => (
<View style={[styles.headerBar, props.shouldShowBorderBottom && styles.borderBottom]}>
<View style={[
styles.dFlex,
styles.flexRow,
styles.alignItemsCenter,
styles.flexGrow1,
styles.justifyContentBetween,
styles.overflowHidden,
]}
>
{props.shouldShowBackButton && (
<Tooltip text={props.translate('common.back')}>
<TouchableOpacity
onPress={() => {
if (VirtualKeyboard.isOpen()) {
Keyboard.dismiss();
}
props.onBackButtonPress();
}}
style={[styles.touchableButtonImage]}
>
<Icon src={Expensicons.BackArrow} />
</TouchableOpacity>
</Tooltip>
)}
<Header
title={props.title}
subtitle={props.stepCounter && props.shouldShowStepCounter ? props.translate('stepCounter', props.stepCounter) : props.subtitle}
/>
<View style={[styles.reportOptions, styles.flexRow, styles.pr5]}>
{
props.shouldShowDownloadButton && (
<Tooltip text={props.translate('common.download')}>

<TouchableOpacity
onPress={props.onDownloadButtonPress}
style={[styles.touchableButtonImage]}
>
<Icon src={Expensicons.Download} />
</TouchableOpacity>
</Tooltip>
)
}

{props.shouldShowGetAssistanceButton
&& (
<Tooltip text={props.translate('getAssistancePage.questionMarkButtonTooltip')}>
<TouchableOpacity
onPress={() => Navigation.navigate(ROUTES.getGetAssistanceRoute(props.guidesCallTaskID))}
style={[styles.touchableButtonImage, styles.mr0]}
accessibilityRole="button"
accessibilityLabel={props.translate('getAssistancePage.questionMarkButtonTooltip')}
>
<Icon src={Expensicons.QuestionMark} />
</TouchableOpacity>
</Tooltip>
)}

{props.shouldShowThreeDotsButton && (
<ThreeDotsMenu
menuItems={props.threeDotsMenuItems}
onIconPress={props.onThreeDotsButtonPress}
iconStyles={[styles.mr0]}
anchorPosition={props.threeDotsAnchorPosition}
class HeaderWithCloseButton extends Component {
constructor(props) {
super(props);

this.triggerButtonCompleteAndDownload = this.triggerButtonCompleteAndDownload.bind(this);
}

/**
* Method to trigger parent onDownloadButtonPress to download the file
* and toggleDelayButtonState to set button state and revert it after sometime.
*/
triggerButtonCompleteAndDownload() {
Copy link
Contributor

Choose a reason for hiding this comment

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

missing js docs

if (this.props.isDelayButtonStateComplete) {
return;
}

this.props.onDownloadButtonPress();
this.props.toggleDelayButtonState(true);
}

render() {
return (
<View style={[styles.headerBar, this.props.shouldShowBorderBottom && styles.borderBottom]}>
<View style={[
styles.dFlex,
styles.flexRow,
styles.alignItemsCenter,
styles.flexGrow1,
styles.justifyContentBetween,
styles.overflowHidden,
]}
>
{this.props.shouldShowBackButton && (
<Tooltip text={this.props.translate('common.back')}>
<Pressable
onPress={() => {
if (VirtualKeyboard.isOpen()) {
Keyboard.dismiss();
}
this.props.onBackButtonPress();
}}
style={[styles.touchableButtonImage]}
>
<Icon src={Expensicons.BackArrow} />
</Pressable>
</Tooltip>
)}
<Header
title={this.props.title}
subtitle={this.props.stepCounter && this.props.shouldShowStepCounter ? this.props.translate('stepCounter', this.props.stepCounter) : this.props.subtitle}
/>
)}

{props.shouldShowCloseButton
&& (
<Tooltip text={props.translate('common.close')}>
<TouchableOpacity
onPress={props.onCloseButtonPress}
style={[styles.touchableButtonImage, styles.mr0]}
accessibilityRole="button"
accessibilityLabel={props.translate('common.close')}
>
<Icon src={Expensicons.Close} />
</TouchableOpacity>
</Tooltip>
)}
<View style={[styles.reportOptions, styles.flexRow, styles.pr5]}>
{
this.props.shouldShowDownloadButton && (
<Tooltip text={this.props.translate('common.download')}>

<Pressable
onPress={this.triggerButtonCompleteAndDownload}
style={[styles.touchableButtonImage]}
>
<Icon
src={Expensicons.Download}
fill={StyleUtils.getIconFillColor(getButtonState(false, false, this.props.isDelayButtonStateComplete))}
/>
</Pressable>
</Tooltip>
)
}

{this.props.shouldShowGetAssistanceButton
&& (
<Tooltip text={this.props.translate('getAssistancePage.questionMarkButtonTooltip')}>
<Pressable
onPress={() => Navigation.navigate(ROUTES.getGetAssistanceRoute(this.props.guidesCallTaskID))}
style={[styles.touchableButtonImage, styles.mr0]}
accessibilityRole="button"
accessibilityLabel={this.props.translate('getAssistancePage.questionMarkButtonTooltip')}
>
<Icon src={Expensicons.QuestionMark} />
</Pressable>
</Tooltip>
)}

{this.props.shouldShowThreeDotsButton && (
<ThreeDotsMenu
menuItems={this.props.threeDotsMenuItems}
onIconPress={this.props.onThreeDotsButtonPress}
iconStyles={[styles.mr0]}
anchorPosition={this.props.threeDotsAnchorPosition}
/>
)}

{this.props.shouldShowCloseButton
&& (
<Tooltip text={this.props.translate('common.close')}>
<Pressable
onPress={this.props.onCloseButtonPress}
style={[styles.touchableButtonImage, styles.mr0]}
accessibilityRole="button"
accessibilityLabel={this.props.translate('common.close')}
>
<Icon src={Expensicons.Close} />
</Pressable>
</Tooltip>
)}
</View>
</View>
</View>
</View>
</View>
);
);
}
}

HeaderWithCloseButton.propTypes = propTypes;
HeaderWithCloseButton.defaultProps = defaultProps;
HeaderWithCloseButton.displayName = 'HeaderWithCloseButton';

export default withLocalize(HeaderWithCloseButton);
export default compose(
withLocalize,
withDelayToggleButtonState,
)(HeaderWithCloseButton);
Loading