-
Notifications
You must be signed in to change notification settings - Fork 129
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
[Feature] Add shortcut keys to theme dev #4830
base: main
Are you sure you want to change the base?
Conversation
Coverage report
Show files with reduced coverage 🔻
Test suite run success1946 tests passing in 885 suites. Report generated by 🧪jest coverage report action from 45d6424 |
6a9b244
to
989d98e
Compare
@@ -113,7 +142,7 @@ export function renderLinks(store: string, themeId: string, host = DEFAULT_HOST, | |||
body: [ | |||
{ | |||
list: { |
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.
When using the theme delete
command, shortcut keys, such as (y) for yes, are put before the description. We opted to put it after for readability.
@@ -113,7 +142,7 @@ export function renderLinks(store: string, themeId: string, host = DEFAULT_HOST, | |||
body: [ | |||
{ | |||
list: { | |||
title: {bold: 'Preview your theme'}, | |||
title: chalk.bold('Preview your theme ') + chalk.cyan('(t)'), |
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.
Due to constraints on the usability of outputContent
and outputToken
, we were unable to add colour to the list of commands. Using chalk which is already available in the repo allows us to do so.
process.stdin.setRawMode(true) | ||
} | ||
|
||
process.stdin.on('keypress', (_str, key) => { |
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.
checking for ctrl + c
at the beginning ensures we kill the process. We need to have stdin
set to rawMode
as it looks character by character and doesn't require hitting the enter key.
a895e5c
to
ea14203
Compare
This comment has been minimized.
This comment has been minimized.
ea14203
to
838bb83
Compare
838bb83
to
10fa708
Compare
} | ||
} | ||
|
||
renderLinks(urls) | ||
if (options.open) { |
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.
Test:
- Pass 'open'
- Mock openURL to throw an error
- Validate that
handleOpenURLError
is called by checking if renderWarning is called
This commit adds shortcut keys to the theme dev CLI service. The keys are as follows: e - open theme editor t - preview your theme locally p - preview your theme (share) g - preview gift cards
10fa708
to
45d6424
Compare
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.
Thank you, @EvilGenius13 🔥 🚀 Amazing stuff! I've left only one minor comment :)
renderSuccess({ | ||
body: [ | ||
{ | ||
list: { | ||
title: {bold: 'Preview your theme'}, | ||
items: [localUrl], | ||
title: chalk.bold('Preview your theme ') + chalk.cyan('(t)'), |
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 believe we could follow the CLI-kit standards to present the shortcuts. Thus, this line could be presented as:
title: [{bold: 'Preview your theme'}, {subdued: '(t)'}],
or:
title: [{bold: 'Preview your theme'}, {command: '(t)'}],
I believe that subdued
fits a bit more in this context. While it's a bit tempting to be opinionated about the color, it's nice to use CLI-kit standard colors, so we may make the CLI more cohesive across commands :)
Do you suffer from long logs? Do you accidentally close your browser window and have to scroll all the way up in your terminal to find the theme dev links? Look no further than the addition of shortcut keys!
WHY are these changes introduced?
Closes #4713
Closes https://github.com/Shopify/develop-advanced-edits/issues/427
With the amount of logs
theme dev
generates, it can be difficult to find the original store links such as previewing your theme, or opening the theme editor. Having shortcut keys is a simple way to open the links without needing to physically click on them in the terminal.WHAT is this pull request doing?
Add shortcut keys to the
theme dev
commandWe look for keypresses, specifically:
t
- opens the theme locallye
- opens the theme editorp
- opens the share previewg
- opens the gift cards previewwe also look for
ctrl
+c
to exit the process (current behaviour)How to test your changes?
Pull down the branch
Build the branch
Run the
theme dev
commandTry out the shortcut keys. Click some links, save some files, and see that it does not affect anything else running,
ctrl+c
out to confirm you can exit the same as before.Post-release steps
Measuring impact
How do we know this change was effective? Please choose one:
Checklist