Contributor Author

@DanTup I don't have permission to request a review, please review :)

Member

Choose a reason for hiding this comment

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

Member

Oh, maybe we should add a mobile icon for "Create Android Emulator"?

Contributor Author

It might be more clear to only show icons for available devices and keep the commands as is

Before open

After open

Member

Yeah, I think you're right - although the alignment looks a bit weird. Is there any blank/spacer we could use so the text all lines up? If not, then I'll merge as-is. Thanks!

Contributor Author

There isn't, but just in case, here's what it looks like with icons:
. WDYT?

Member

I think that looks ok, but I wouldn't say miles better so I'll leave it to you.

It might be better if we could have a separator like shown here:

But I can't find the code doing that - I'm not sure if it's VS Code doing it from some special API for notebooks, or if we can do this ourselves 🤔

Contributor Author

It's coming next release apparently! I filed #3655. I'll go with icons

Member

Great!

There's a failing test on the PR:

  2) device_manager
       overrides real emulators with custom definitions:
      AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
+ actual - expected
+ '$(play) Start My emulator override'
- 'Start My emulator override'
      + expected - actual
      -$(play) Start My emulator override
      +Start My emulator override

To run, select Flutter or Flutter LSP tests in the debug pane (you can also change the test to it.only( to run just that test) - although the fix is probably fairly clear :-)

Thanks!

Member

Thanks!