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.

Sign up for GitHub

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

Jump to bottom

[Feature] Add shortcut keys to theme dev #4830

Merged

EvilGenius13 merged 1 commit into main from add-shortcut-keys-to-theme-dev

Nov 22, 2024

Contributor

EvilGenius13 commented Nov 11, 2024 •

edited

Loading

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 command
We look for keypresses, specifically:
t - opens the theme locally
e - opens the theme editor
p - opens the share preview
g - opens the gift cards preview
we also look for ctrl + c to exit the process (current behaviour)

Screenshot 2024-11-21 at 4 51 40 PM

How to test your changes?

Pull down the branch
Build the branch
Run the theme dev command
Try 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:

n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
Existing analytics will cater for this addition
PR includes analytics changes to measure impact

Checklist

I've considered possible cross-platform impacts (Mac, Linux, Windows)
I've considered possible documentation changes

EvilGenius13 requested a review from karreiro

November 11, 2024 19:02

Contributor

github-actions bot commented Nov 11, 2024 •

edited

Loading

Coverage report

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	74.54% (+0.02% 🔼)	8543/11461
🟡	Branches	70.35% (+0.03% 🔼)	4171/5929
🟡	Functions	74.06%	2241/3026
🟡	Lines	75.08% (+0.02% 🔼)	8079/10760

Test suite run success

1946 tests passing in 885 suites.

Report generated by 🧪jest coverage report action from 057e2c6

EvilGenius13 force-pushed the add-shortcut-keys-to-theme-dev branch 2 times, most recently from 6a9b244 to 989d98e Compare

November 20, 2024 19:20

EvilGenius13 commented

View reviewed changes

packages/theme/src/cli/services/dev.ts

                   body: [
                     {
                       list: {

Contributor Author

EvilGenius13 Nov 20, 2024

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.

EvilGenius13 commented

View reviewed changes

packages/theme/src/cli/services/dev.ts

    
            @@ -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)'),

Contributor Author

EvilGenius13 Nov 20, 2024

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.

EvilGenius13 commented

View reviewed changes

packages/theme/src/cli/services/dev.ts

+                  process.stdin.setRawMode(true)
+                }
+                process.stdin.on('keypress', (_str, key) => {

Contributor Author

EvilGenius13 Nov 20, 2024

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.

EvilGenius13 force-pushed the add-shortcut-keys-to-theme-dev branch 3 times, most recently from a895e5c to ea14203 Compare

November 20, 2024 20:37

EvilGenius13 marked this pull request as ready for review

November 20, 2024 20:43

EvilGenius13 requested review from a team as code owners

November 20, 2024 20:43

EvilGenius13 requested a review from jamesmengo

November 20, 2024 20:43

This comment has been minimized.

Sign in to view

EvilGenius13 force-pushed the add-shortcut-keys-to-theme-dev branch from ea14203 to 838bb83 Compare

November 20, 2024 20:49

graphite-app bot reviewed

View reviewed changes

packages/theme/src/cli/services/dev.ts Outdated Show resolved Hide resolved

lukeh-shopify reviewed

View reviewed changes

packages/theme/src/cli/services/dev.ts Outdated Show resolved Hide resolved

packages/theme/src/cli/services/dev.ts Outdated Show resolved Hide resolved

EvilGenius13 force-pushed the add-shortcut-keys-to-theme-dev branch from 838bb83 to 10fa708 Compare

November 20, 2024 21:55

jamesmengo reviewed

View reviewed changes

packages/theme/src/cli/services/dev.ts Show resolved Hide resolved

jamesmengo approved these changes

View reviewed changes

EvilGenius13 force-pushed the add-shortcut-keys-to-theme-dev branch from 10fa708 to 45d6424 Compare

November 21, 2024 21:37

karreiro approved these changes

View reviewed changes

Contributor

karreiro left a comment

Thank you, @EvilGenius13 🔥 🚀 Amazing stuff! I've left only one minor comment :)

packages/theme/src/cli/services/dev.ts

                 renderSuccess({
                   body: [
                     {
                       list: {
-                        title: {bold: 'Preview your theme'},
-                        items: [localUrl],
+                        title: chalk.bold('Preview your theme ') + chalk.cyan('(t)'),

Contributor

karreiro Nov 22, 2024

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 :)

Contributor Author

EvilGenius13 Nov 22, 2024 •

edited

Loading

After doing some digging, this won't work for labels as they expect a string. We will have to make some changes in cli-kit. I have made a follow up issue here to make that fix. I will ship for now and circle back.

EvilGenius13 added this pull request to the merge queue

github-merge-queue bot removed this pull request from the merge queue due to failed status checks

EvilGenius13 force-pushed the add-shortcut-keys-to-theme-dev branch from 45d6424 to 32b8da5 Compare

November 22, 2024 16:21


          Add shortcut keys to theme dev

057e2c6

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

EvilGenius13 force-pushed the add-shortcut-keys-to-theme-dev branch from 32b8da5 to 057e2c6 Compare

November 22, 2024 17:29

EvilGenius13 added this pull request to the merge queue

Merged via the queue into main with commit 7df6667

27 checks passed

EvilGenius13 deleted the add-shortcut-keys-to-theme-dev branch

November 22, 2024 17:43

shopify-shipit bot temporarily deployed to nightly

November 23, 2024 14:33

Inactive

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet