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.

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

Add interactive mode with keyboard commands #326

Merged
merged 11 commits into from
Jul 28, 2017
Merged

Add interactive mode with keyboard commands #326

merged 11 commits into from
Jul 28, 2017

Conversation

fson
Copy link
Contributor

@fson fson commented Jul 24, 2017

yarn start command now starts react-native-scripts in an interactive mode.

screen shot 2017-07-24 at 22 43 37

Supported commands:

  • Open Android device or emulator
  • Open iOS emulator
  • Display QR code
  • Restart packager
  • Restart packager and clear cache
  • Toggle development mode.

screen shot 2017-07-24 at 22 44 41

screen shot 2017-07-24 at 22 44 54

screen shot 2017-07-24 at 22 54 00

@fson fson requested a review from brentvatne July 24, 2017 19:55
@brentvatne
Copy link
Member

brentvatne commented Jul 24, 2017

hey @fson this looks great so far! so excited for it

a bit of feedback:

  • can we show QR code without clearing the screen?
  • also would be good for open ios / android to not clear screen
  • maybe make clear screen an option? (given that normal clear screen doesn't work with interactive prompt)
  • ask people to close and reopen the app in expo client when they toggle dev/production as this is currently required (Pick up change to dev mode when reloading the app expo#413)
  • condense some items into one line (see example below)
  • add a blank newline after the prompt instructions so separation from logs is clear (see example below)
  • remove the "Usage" line

EDIT: nevermind the ones I crossed out, changed my mind

Or enter this address in the Expo app's search bar:

  exp://10.0.9.200:19000

Your phone will need to be on the same local network as this computer.

For links to install the Expo app, please visit https://expo.io.

Logs from serving your app will appear here. Press Ctrl+C at any time to stop.

 › Press a to open Android device or emulator, or i to open iOS emulator.
 › Press q to display QR code.
 › Press r to restart packager, or R to restart packager and clear cache.
 › Press d to toggle development mode. (current mode: production)

2:00:25 PM: Running app on iPhone Simulator

Copy link
Member

@brentvatne brentvatne left a comment

Choose a reason for hiding this comment

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

oops, left comment without explicit review

@brentvatne
Copy link
Member

oh, also, we should remove the ios option when you're on windows

@fson
Copy link
Contributor Author

fson commented Jul 25, 2017

Great suggestions, thanks for the feedback @brentvatne. I've implemented the changes.

I think condensing some commands together was also a good idea, as I was a bit concerned that the instructions would not fit nicely on a single screen anymore, so I've implemented that one too (looks good IMHO). I also removed one empty line (before the "For links to install the Expo app..." line) to better fit all the information on the screen at once. Let me know what you think!
screen shot 2017-07-25 at 12 57 02
screen shot 2017-07-25 at 13 16 19

@brentvatne
Copy link
Member

looks awesome! only thing missing here is highlighting on the R for restart with clear cache

screen shot 2017-07-25 at 11 14 00

@fson
Copy link
Contributor Author

fson commented Jul 25, 2017

Oops, good catch – R is highlighted now.

@brentvatne
Copy link
Member

@fson - two more things:

  1. have you tested on windows? which shell?
  2. can we add a flag like --no-interactive? if you can think of a better name that works too

@fson
Copy link
Contributor Author

fson commented Jul 26, 2017

@brentvatne:

  1. I'll test on Windows tomorrow.
  2. What would be the use case for the --no-interactive flag? Is it just to make sure the start script still works e.g. when spawned as a subprocess and/or the output is piped to another process? If so, maybe it's enough that we check process.stdout.isTTY and only enable the interactivity when it's true? (It seems that the typeof stdin.setRawMode === 'function' check I have does already something similar, but I'll have to check that.) We should also not print the instructions when the interactive mode is not enabled.

@brentvatne
Copy link
Member

@fson

  1. cool thanks :)
  2. some people don't like fancy new things :O also, we haven't put this to the test for a long period of time while building a project so it's hard to say what nits we might have about it, so it would be good to let people go back to old behavior if they prefer it

@fson
Copy link
Contributor Author

fson commented Jul 27, 2017

Tested on Windows 10:

  • Using cmd.exe: otherwise works but the entered characters are displayed on the screen, which doesn't look very nice:
    screen shot 2017-07-27 at 18 43 38
  • Using Git Bash: character input doesn't work, can't exit :O

I'll try to fix these.

`npm start -- --no-interactive` runs the server without interactivity.
@fson
Copy link
Contributor Author

fson commented Jul 27, 2017

npm start -- --no-interactive now starts the server without interactivity.

@brentvatne
Copy link
Member

@fson - ah we don't support git bash currently, it doesn't even work with popular libs like inquirer 😊

@brentvatne
Copy link
Member

@fson - just powershell and cmd.exe

@fson
Copy link
Contributor Author

fson commented Jul 27, 2017

@brentvatne Ah, good ☺️ Then I'll just fix the output on cmd.exe and test on PowerShell.

@fson
Copy link
Contributor Author

fson commented Jul 27, 2017

PowerShell also displays the input (see d in the beginning) and chalk also highlights a bit more than it should:
screen shot 2017-07-27 at 19 49 28

fson added 4 commits July 28, 2017 14:49
The `readline` setup on Windows that nicely suppresses the annoying
"Terminate batch job (Y/N)" prompt when pressing Ctrl+c also caused
the keyboard commands on interactive mode to be printed to stdout.
Fix this by piping the `readline` output to a MuteStream.
Creating the readline interface on Windows to listen for Ctrl+c caused
the text "Stopping packager" to be printed twice. In the interactive
mode stopping the packager is handled by the keypress listener set
by the interactive mode, so the readline interface is not necessary.
@fson
Copy link
Contributor Author

fson commented Jul 28, 2017

I found the reason why the input characters were printed to the console on Windows and fixed it.

Now the only problem seems to be the bold chalk style not resetting on Windows (chalk/chalk#145). I'll try to work around it by adding some chalk.reset() to the output.

Work around the bold style not being closed on Windows by adding reset
styles after each bold style.
@fson
Copy link
Contributor Author

fson commented Jul 28, 2017

Now the text styles on Windows are fixed too:
screen shot 2017-07-28 at 16 12 32
screen shot 2017-07-28 at 16 14 38

@brentvatne
Copy link
Member

weird! I wonder if there is a way to work around that @fson?

@fson
Copy link
Contributor Author

fson commented Jul 28, 2017

I fixed the bold style overflowing to the following text by adding chalk.reset() after any bold texts. Looks good to me now. (Windows doesn't seem to support the dim style used in the instructions, but it still looks alright.)

@brentvatne
Copy link
Member

thanks @fson this looks great

@brentvatne brentvatne merged commit 549dbe8 into master Jul 28, 2017
@fson fson deleted the interactive branch July 28, 2017 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants