-
Notifications
You must be signed in to change notification settings - Fork 954
[WIP] Add a way to select Android User to let Flipper working on devices with Work Profile #4606
base: main
Are you sure you want to change the base?
Changes from 5 commits
135c7b1
9f9c2cc
38792c6
7afef0b
1effdd5
b1a24a0
0068dbb
95d3830
67e0353
99497c4
8f823ec
a49ff37
3bd1b6f
a152f29
375d749
7d5a978
ea4c687
ca85b69
5d1f6b1
920ba5c
2a02952
45186b0
70e2414
78c378c
6b26be3
33295bd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,21 +7,22 @@ | |
* @format | ||
*/ | ||
|
||
import React, {Component, useContext} from 'react'; | ||
import React, {Component, useContext, useState} from 'react'; | ||
import {Radio} from 'antd'; | ||
import {updateSettings, Action} from '../reducers/settings'; | ||
import { | ||
Action as LauncherAction, | ||
updateLauncherSettings, | ||
} from '../reducers/launcherSettings'; | ||
import {connect} from 'react-redux'; | ||
import {connect, useSelector} from 'react-redux'; | ||
import {State as Store} from '../reducers'; | ||
import {flush} from '../utils/persistor'; | ||
import ToggledSection from './settings/ToggledSection'; | ||
import { | ||
FilePathConfigField, | ||
ConfigText, | ||
URLConfigField, | ||
ComboBoxConfigField, | ||
} from './settings/configFields'; | ||
import KeyboardShortcutInput from './settings/KeyboardShortcutInput'; | ||
import {isEqual, isMatch, isEmpty} from 'lodash'; | ||
|
@@ -40,8 +41,9 @@ import { | |
_NuxManagerContext, | ||
NUX, | ||
} from 'flipper-plugin'; | ||
import {getRenderHostInstance} from 'flipper-frontend-core'; | ||
import {BaseDevice, getRenderHostInstance} from 'flipper-frontend-core'; | ||
import {loadTheme} from '../utils/loadTheme'; | ||
import {getActiveDevice} from '../selectors/connections'; | ||
|
||
type OwnProps = { | ||
onHide: () => void; | ||
|
@@ -120,6 +122,7 @@ class SettingsSheet extends Component<Props, State> { | |
const { | ||
enableAndroid, | ||
androidHome, | ||
androidUserId, | ||
enableIOS, | ||
enablePhysicalIOS, | ||
enablePrefetching, | ||
|
@@ -180,6 +183,17 @@ class SettingsSheet extends Component<Props, State> { | |
}); | ||
}} | ||
/> | ||
<AndroidUserIdField | ||
defaultValue={androidUserId} | ||
onChange={(v) => { | ||
this.setState({ | ||
updatedSettings: { | ||
...this.state.updatedSettings, | ||
androidUserId: v, | ||
}, | ||
}); | ||
}} | ||
/> | ||
</ToggledSection> | ||
<ToggledSection | ||
label="iOS Developer" | ||
|
@@ -499,6 +513,46 @@ export default connect<StateFromProps, DispatchFromProps, OwnProps, Store>( | |
{updateSettings, updateLauncherSettings}, | ||
)(withTrackingScope(SettingsSheet)); | ||
|
||
function AndroidUserIdField(props: { | ||
defaultValue: string; | ||
onChange: (path: string) => void; | ||
}) { | ||
const activeDevice: BaseDevice = useSelector(getActiveDevice); | ||
const [users, setUsers] = useState([] as {id: string; name: string}[]); | ||
|
||
activeDevice | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Best run shell commands in an effect, with the current setup the command is fired on every render There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As said on comments, I don't know React. Please feel free to edit that part, as I don't have knowledge about these "useEffect" things. Just a warn: when you say "the command is fired on every render", it may be fine. Best would be to run this command each time selected device change (and user is opening settings sheet) or you could have bad users ids shown. |
||
.executeShell('pm list users') | ||
.then((result) => { | ||
const users = result | ||
.match(/(?<=UserInfo{)(.*?)(?=})/g) | ||
?.map((userInfo) => { | ||
const infos = userInfo.split(':'); | ||
return {id: infos[0], name: `${infos[0]} (${infos[1]})`}; | ||
}); | ||
setUsers(users || []); | ||
}) | ||
.catch((error) => console.error(error)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add some more details to the error message? Also let's start with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This command should not fail. If this command fails there is chances that every shell command will. If I have a look on Flipper source code where executeShell is called, errors are not even managed. For example, on plugins/public/cpi/index.tsx const chipname = await executeShell('getprop ro.chipname'); There is no try/catch. With these informations in mind, what do you think about changes I should work on this? Just a warn instead of an error? |
||
|
||
if (users.length === 0) { | ||
return ( | ||
<ConfigText | ||
content={ | ||
'Loading... Please be sure to connect a device to show available users.' | ||
}></ConfigText> | ||
); | ||
} | ||
|
||
return ( | ||
<ComboBoxConfigField | ||
label="Android User" | ||
resetValue={getRenderHostInstance().serverConfig.settings.androidUserId} | ||
options={users} | ||
defaultValue={props.defaultValue} | ||
onChange={props.onChange} | ||
/> | ||
); | ||
} | ||
|
||
function ResetTooltips() { | ||
const nuxManager = useContext(_NuxManagerContext); | ||
|
||
|
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.
could we pass in less here than the entire flipperServer? For example just the settings object?
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 can do that yes 👍
What's your though about the more suitable thing to pass in?
You are suggesting the settings object, but the minimal one would be the userId. Indeed, the only reason why flipperServer is pass in is to read "this.flipperServer.config.settings.androidUserId"