Add button for dev menu #42
Conversation
|
@jakub-gonet is attempting to deploy a commit to the software-mansion Team on Vercel. To accomplish this, @jakub-gonet needs to request access to the Team. Afterwards, an owner of the Team is required to accept their membership request. If you're already a member of the respective Vercel Team, make sure that your Personal Vercel Account is connected to your GitHub account. |
| if (platform === Platform.IOS) { | ||
| this.devtools.send("rnp_iosDevMenu"); | ||
| } else { | ||
| await (this.device as AndroidEmulatorDevice).openDevMenu(); | ||
| } |
There was a problem hiding this comment.
- This should live in
metro.tsideally. - Can't be method on
DeviceBasebecause needs devtools to send an event – no need to add it into method signature nor class state. - Polimorphism failed me here but
ifnever disappoint.
| @@ -130,6 +130,14 @@ export function PreviewAppWrapper({ children, ...rest }) { | |||
| newRoute && push(newRoute); | |||
| }); | |||
|
|
|||
| agent._bridge.addListener("rnp_iosDevMenu", (_payload) => { | |||
There was a problem hiding this comment.
I changed the prefix we use recently for commands to RNIDE_ IIRC. Can you update it here
| <SettingsDropdown project={project} disabled={devicesNotFound}> | ||
| <SettingsDropdown | ||
| project={project} | ||
| isDeviceRunning={projectState.status === "running"} |
There was a problem hiding this comment.
maybe not worth adding extra parameter given we already pass project down
There was a problem hiding this comment.
We'd need to call getProjectState() which returns a promise which is harder to work with so I opted to use the status if we have it there.
| @@ -281,6 +281,13 @@ export class Project implements Disposable, MetroDelegate, ProjectInterface { | |||
| this.deviceSession?.openNavigation(navigationItemID); | |||
| } | |||
|
|
|||
| public async openDevMenu() { | |||
| const currentDevice = this.projectState.selectedDevice; | |||
| if (currentDevice !== undefined) { | |||
There was a problem hiding this comment.
why do we need this check? Seems like the device session implementation should deal with this type of edge cases and here should should just delegate it to it
There was a problem hiding this comment.
I had a hard time structuring this code. We'd need to pass selectedDevice to device session. We could also add platform() callback to DeviceBase which would be really convenient but it kinda defeats the purpose of subclassing.
Initially I thought this wasn't the best idea but right now seems like not bad solution.
There was a problem hiding this comment.
The best solution would be to create openDevMenu in BaseDevice but in case of iOS it'd need devtools which I don't like.
jakub-gonet
force-pushed
the
jgonet/dev-menu
branch
from
c621475
to
125f678
Compare
| @@ -37,6 +37,10 @@ export class AndroidEmulatorDevice extends DeviceBase { | |||
| super(); | |||
| } | |||
|
|
|||
| public platform(): Platform { | |||
jakub-gonet
force-pushed
the
jgonet/dev-menu
branch
from
125f678
to
d8e2f11
Compare
Adds a button in IDE settings that opens dev menu. On iOs it uses native module to open it, on Android we spawn adb process and send
KEYCODE_MENU(code 82) to the emulator.Resolves #30.
android.mov
ios.mov