-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Open a test tools modal with 5 taps on dev #13900
Merged
Merged
Changes from 20 commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
ede8ea5
Initial quintuple tap gesture on header view
neil-marcellini 83f8d9a
Move quintuple tap gesture into the screen wrapper
neil-marcellini a6d7586
Add a small centered modal type
neil-marcellini e5c4141
Use Onyx to open and close the test tools modal
neil-marcellini 3065c6c
Blank test tools modal in the screen wrapper
neil-marcellini eb2573c
Create a test tools modal
neil-marcellini 3d6fbc7
Open the test tools menu on 5 taps in dev only
neil-marcellini 933d994
Only render the test tools modal on dev
neil-marcellini bea47c8
Fix the gesture onEnd callback
neil-marcellini c762879
Fix iOS by running callbacks on the JS thread
neil-marcellini 73c635c
Somewhat fix the iOS modal
neil-marcellini 9d18e53
Merge branch 'main' into neil-test-tool-gesture
neil-marcellini 75d30f7
Min width for mobile test tool modal
neil-marcellini 2a99b12
Merge branch 'main' into neil-test-tool-gesture
neil-marcellini b9a901b
lint
neil-marcellini ffd6288
Clean up
neil-marcellini 257e1b2
Add missing prop type
neil-marcellini 40105a4
Standardize on TestToolsModal name
neil-marcellini 0d3c1c6
Fix test tools modal after merge
neil-marcellini f598264
Remove extra top margin
neil-marcellini f3fb8ab
Throttle toggling test tools modal for mWeb Chrome
neil-marcellini eca2c73
Use constant for test tools modal throttle time
neil-marcellini File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
import React from 'react'; | ||
import PropTypes from 'prop-types'; | ||
import {withOnyx} from 'react-native-onyx'; | ||
import {View} from 'react-native'; | ||
import ONYXKEYS from '../ONYXKEYS'; | ||
import Modal from './Modal'; | ||
import CONST from '../CONST'; | ||
import * as App from '../libs/actions/App'; | ||
import TestToolMenu from './TestToolMenu'; | ||
import styles from '../styles/styles'; | ||
|
||
const propTypes = { | ||
/** Whether the test tools modal is open */ | ||
isTestToolsModalOpen: PropTypes.bool, | ||
}; | ||
|
||
const defaultProps = { | ||
isTestToolsModalOpen: false, | ||
}; | ||
|
||
const TestToolsModal = props => ( | ||
<Modal | ||
isVisible={props.isTestToolsModalOpen} | ||
type={CONST.MODAL.MODAL_TYPE.CENTERED_SMALL} | ||
onClose={App.closeTestToolsModal} | ||
> | ||
<View style={[styles.settingsPageBody, styles.p5]}> | ||
<TestToolMenu /> | ||
</View> | ||
</Modal> | ||
); | ||
|
||
TestToolsModal.propTypes = propTypes; | ||
TestToolsModal.defaultProps = defaultProps; | ||
TestToolsModal.displayName = 'TestToolsModal'; | ||
|
||
export default withOnyx({ | ||
modal: { | ||
key: ONYXKEYS.MODAL, | ||
}, | ||
isTestToolsModalOpen: { | ||
key: ONYXKEYS.IS_TEST_TOOLS_MODAL_OPEN, | ||
}, | ||
})(TestToolsModal); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,10 @@ export default { | |
minWidth: '25%', | ||
}, | ||
|
||
mnw120: { | ||
minWidth: 120, | ||
}, | ||
|
||
w50: { | ||
width: '50%', | ||
}, | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@neil-marcellini Should this be here or still an outer layer?
I tried the taps on Login page and it didn't work? Now I am not sure if we would even need it there, but we do have controls like sign-in and language change which cannot be tested offline if the Test modal won't show up.
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.
That's a good point. It will be good to place it in a more outer layer.
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.
I added this to the ScreenWrapper since it appears to wrap almost every page. The public screens that have content other than a loading indicator, such as the SetPasswordPage and the SignInPage, do not use the screen wrapper.
I could try to use the ScreenWrapper on these pages but I would have to test how that works. Alternatively I could create another wrapper component specifically for this gesture and use it on these public pages as well in the screen wrapper.
I think it's relatively rare to test changing offline state on the public screens, so maybe I could implement that with a follow up PR sometime next week? Does that work for you guys?
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.
Yeah a follow up PR sounds okay to me.