Allow user to speed up sites on Windows programmatically - #211
Conversation
13fd65c to
d0ed46c
Compare
d0ed46c to
2306c3f
Compare
| [ stat: string ]: number; | ||
| }; | ||
| }; | ||
| promptWindowsSpeedUpResult?: PromptWindowsSpeedUpResult; |
There was a problem hiding this comment.
The result is persisted to show only once the prompt in the onboarding.
| const exeDir = path.dirname( app.getPath( 'exe' ) ); | ||
| exePath = path.join( exeDir, '..', 'app-*', exeFilename ); | ||
| } | ||
| const command = `PowerShell -NoProfile -ExecutionPolicy Bypass -Command "Add-MpPreference -ExclusionProcess ${ exePath }"`; |
There was a problem hiding this comment.
For reference, here's the Windows documentation for this command.
| let exePath = app.getPath( 'exe' ); | ||
| // When the app is packaged, the exe path points to "%appdata%\Local\studio\app-{app-version}\Studio.exe". | ||
| // To avoid updating this configuration on each update, we use a wilcard in the path to include all versions. | ||
| if ( app.isPackaged ) { |
There was a problem hiding this comment.
The app is not packaged when it's executed from the Electron shell located in node_modules. This value is also used to distinguish development and production environments.
| buttons, | ||
| title: __( 'Want to speed up sites?' ), | ||
| message: __( | ||
| "If the Real-Time Protection Service of Windows Defender is enabled on your machine, it may slow down the process of creating and starting a site.\n\nTo enhance site speed, it's recommended adjusting the configuration accordingly.\n\nThe app can do this automatically for you, or alternatively, you can follow the documentation." |
There was a problem hiding this comment.
I'd appreciate any feedback on the messaging. The content is mostly based on the Documentation.
There was a problem hiding this comment.
Could we maybe add the Needs Copy Review label or ping @\Automattic/editorial to get some extra help from the Publications team? I don't feel overly confident in my copy abilities, but feel the adjusting the configuration accordingly line is perhaps a bit too vague in this context.
Maybe it could be updated to something like the following?
For optimal performance, we recommend disabling this service for the Studio app.
There was a problem hiding this comment.
Could we maybe add the
Needs Copy Reviewlabel or ping@\Automattic/editorialto get some extra help from the Publications team? I don't feel overly confident in my copy abilities, but feel theadjusting the configuration accordinglyline is perhaps a bit too vague in this context.
Good idea, I've just added the label.
Maybe it could be updated to something like the following?
For optimal performance, we recommend disabling this service for the Studio app.
The idea is to exclude the app from this service, not disabling as Windows Defender is meant to protect user's machine from security threats.
There was a problem hiding this comment.
Oh, that's what I was trying to convey with the for the Studio app part of the suggestion, but see it wasn't clear. 😅 How about something like the following?
For optimal performance, we recommend excluding the Studio app from this service.
I'm sure the Publications team will have some better suggestions than myself, though!
There was a problem hiding this comment.
Oh, that's what I was trying to convey with the
for the Studio apppart of the suggestion, but see it wasn't clear. 😅
Ah, true. I've just re-read it and I found it clearer now 😅 .
[...] How about something like the following?
For optimal performance, we recommend excluding the Studio app from this service.
Sounds good, I'll update the message.
SiobhyB
left a comment
There was a problem hiding this comment.
Thank you for working on this @fluiddot! The happy path works well for me. Before allowing the change via the dialog, it took around 23 seconds to create a new site:
slow.mov
After, it took about 12 seconds:
quicker.mov
I also confirmed that the prompt was only displayed the first time and displays in the Help section. 🙌 I'll add some more thoughts on the copy separately.
| const MANUAL_UPDATE = __( "I'll do it my own by following the documentation." ); | ||
| const AUTOMATIC_UPDATE = __( 'Sounds good, do it for me.' ); | ||
|
|
||
| const buttons = [ NOT_INTERESTED, MANUAL_UPDATE, AUTOMATIC_UPDATE ]; |
There was a problem hiding this comment.
When looking at the dialog, I think it would feel more intuitive for me to see the Sounds good, do it for me option at the top of the list, not the bottom.
I also wondered if it'd make sense for the I'm not interested option to follow the same Don't ask again pattern we use for the demo update and arm64 optimisation dialogs:
Wdyt?
There was a problem hiding this comment.
@matt-west, I'd be interested to hear your perspective on this, too. The three options feel a little cluttered to me:
There was a problem hiding this comment.
When looking at the dialog, I think it would feel more intuitive for me to see the
Sounds good, do it for meoption at the top of the list, not the bottom.
I originally put the buttons in that order but changed them at the last moment because the default response when canceling the dialog is associated with the first element. I thought this order might be following design guidelines, but I've just checked the documentation and it's recommended to have the "Yes" action in the first position:
The "do it" action button(s) should appear as the leftmost buttons. The safe, nondestructive action should appear as the rightmost button.
I'll apply this change.
I also wondered if it'd make sense for the
I'm not interestedoption to follow the sameDon't ask againpattern we use for the demo update and arm64 optimisation dialogs:
This dialog is only prompted automatically once, so I think we wouldn't need to provide a turn-off option. WDYT?
There was a problem hiding this comment.
This dialog is only prompted automatically once, so I think we wouldn't need to provide a turn-off option. WDYT?
Good point, the main motivation behind the suggestion was making I'm not interested less prominent, which I think moving to the last position in the button list would accomplish. 👍
There was a problem hiding this comment.
Do we need to include the option for manually applying the change? I think we could simplify this dialog by removing that option.
My rationale behind adding the option was to provide a middle solution where the user could check the documentation. However, if we foresee that the option won't be helpful to the user and reduces the clarity of the dialog, I'll be happy to remove it. WDYT @matt-west ?
There was a problem hiding this comment.
Okay. Let's be opinionated here and remove that option. Well need to adjust the body copy to reflect this change too.
There was a problem hiding this comment.
Well need to adjust the body copy to reflect this change too.
Originally, I thought of adding a link to the documentation in the body but the native dialog just renders plain text 😞. Do you think we should reference the documentation or simply offering the option to run the action automatically?
There was a problem hiding this comment.
I don't think we need to reference the docs here. If we find that users are confused about what Windows Defender is and how it works, we can add some additional copy to the dialog.
| message: __( | ||
| 'If the Real-Time Protection Service of Windows Defender is enabled on your machine, it may slow down the process of creating and starting a site.\n\nFor optimal performance, we recommend excluding the Studio app from this service.\n\nThe app can do this automatically for you, or alternatively, you can follow the documentation.' | ||
| ), | ||
| cancelId: buttons.indexOf( NOT_INTERESTED ), |
There was a problem hiding this comment.
When closing the dialog, it returns by default the ID 0, we have to set the specific button associated with canceling the process.
|
Hi @fluiddot, I read through the PR and I have a few questions about the modal. Here's the text on the modal:
Questions:
Your answers will help me to come up with an alternative wording. Thanks in advance! |
|
Thank you @kristastevens for helping us with the wording 🙇 !
Real-time protection slows down the execution of PHP files for sites served locally. This applies to all sites created on the Studio app as they are local. So excluding the app in this service speeds up sites in general. However, the most notorious items are creating and starting a site.
I checked different documentation and found references to multiple names: Windows Security, Microsoft Defender, and Windows Defender. On Windows 11, if I navigate to the screen to add exclusions the description says:
I understand that the service is Microsoft Defender and that's part of the OS feature Windows Security. Hence, we could use Microsoft Defender.
Yes, the app will automatically add an exclusion to the Real-time protection to disable this service only for the Studio app. I think it's important to convey that we only disable it for the app, so Microsoft Defender will keep protecting the user's machine. UPDATE: After f3a441a, the message has been changed to:
|
SiobhyB
left a comment
There was a problem hiding this comment.
Going ahead to approve, pending any copy or UI suggestions from Krista or Matt, so as not to block this PR with upcoming AFK. The main flows, including automatically speeding up the site and dismissing the dialog, work great for me. Thanks @fluiddot! 🙌
|
Hi @fluiddot, thanks so much for the clarification. Here's an alternative wording:
|
Great, thanks @kristastevens for your help on the wording. I applied this in af63a6e.
I'd like to note that this PR is related to the Studio app, not WordPress. |
## Related issues Closes ~35 open Dependabot security alerts on the repository. ## How AI was used in this PR Claude identified which Dependabot alerts were fixable via `npm update` (stale lockfile entries within existing semver ranges) vs. those requiring direct dep bumps or overrides, then applied all the easy wins in one shot. ## Proposed Changes Updates transitive and direct dependencies to their patched versions to close the majority of open Dependabot security alerts. All updates are within the existing declared semver ranges of their parent packages — no breaking changes expected. | Package | From | To | Alerts closed | |---------|------|----|---------------| | `fast-uri` | 3.1.0 | 3.1.2 | [#210](https://github.com/Automattic/studio/security/dependabot/210), [#207](https://github.com/Automattic/studio/security/dependabot/207) | | `@xmldom/xmldom` | 0.8.11 | 0.8.13 | [#192](https://github.com/Automattic/studio/security/dependabot/192), [#191](https://github.com/Automattic/studio/security/dependabot/191), [#190](https://github.com/Automattic/studio/security/dependabot/190), [#170](https://github.com/Automattic/studio/security/dependabot/170) | | `brace-expansion` | 1.1.12 | 1.1.15 | [#168](https://github.com/Automattic/studio/security/dependabot/168) | | `qs` | 6.14.0 | 6.15.2 | [#228](https://github.com/Automattic/studio/security/dependabot/228), [#89](https://github.com/Automattic/studio/security/dependabot/89), [#110](https://github.com/Automattic/studio/security/dependabot/110) | | `ip-address` | 10.1.0 | 10.1.1 | [#199](https://github.com/Automattic/studio/security/dependabot/199) | | `basic-ftp` | 5.3.0 | 5.3.1 | [#202](https://github.com/Automattic/studio/security/dependabot/202) | | `ws` | 8.18.0 | 8.21.0 | [#224](https://github.com/Automattic/studio/security/dependabot/224) | | `protobufjs` | 7.5.5 | 7.6.2 | [#221](https://github.com/Automattic/studio/security/dependabot/221), [#220](https://github.com/Automattic/studio/security/dependabot/220), [#217](https://github.com/Automattic/studio/security/dependabot/217), [#216](https://github.com/Automattic/studio/security/dependabot/216), [#219](https://github.com/Automattic/studio/security/dependabot/219), [#218](https://github.com/Automattic/studio/security/dependabot/218), [#215](https://github.com/Automattic/studio/security/dependabot/215), [#225](https://github.com/Automattic/studio/security/dependabot/225) | | `hono` | 4.12.7 | 4.12.23 | [#211](https://github.com/Automattic/studio/security/dependabot/211), [#208](https://github.com/Automattic/studio/security/dependabot/208), [#204](https://github.com/Automattic/studio/security/dependabot/204), [#203](https://github.com/Automattic/studio/security/dependabot/203), [#188](https://github.com/Automattic/studio/security/dependabot/188), [#185](https://github.com/Automattic/studio/security/dependabot/185), [#184](https://github.com/Automattic/studio/security/dependabot/184), [#183](https://github.com/Automattic/studio/security/dependabot/183), [#182](https://github.com/Automattic/studio/security/dependabot/182), [#181](https://github.com/Automattic/studio/security/dependabot/181), [#209](https://github.com/Automattic/studio/security/dependabot/209) | | `@hono/node-server` | 1.19.11 | 1.19.14 | [#180](https://github.com/Automattic/studio/security/dependabot/180) | | `lodash` | 4.17.23 | 4.18.1 | [#172](https://github.com/Automattic/studio/security/dependabot/172), [#171](https://github.com/Automattic/studio/security/dependabot/171) | | `@babel/plugin-transform-modules-systemjs` | 7.27.1 | 7.29.7 | [#212](https://github.com/Automattic/studio/security/dependabot/212) | | `@anthropic-ai/sdk` | ^0.90.0 | ^0.91.1 | [#197](https://github.com/Automattic/studio/security/dependabot/197), [#196](https://github.com/Automattic/studio/security/dependabot/196) | **Remaining open alerts** are blocked by exact-pinned third-party packages (`@php-wasm/*`, `@mariozechner/pi-*`, `electron2appx`) or have no upstream fix (`showdown`). Those require upstream releases to resolve. ## Testing Instructions - [ ] `npm run typecheck` passes - [ ] `npm test` passes - [ ] App starts and sites load normally ## Pre-merge Checklist - [x] No direct dependency version constraints changed (all updates are lockfile-only except `@anthropic-ai/sdk` range bump) - [x] `@wp-playground/*` and `@php-wasm/*` pins untouched
Note
This PR is a proposal to solve 7013-gh-Automattic/dotcom-forge. I'd appreciate any feedback on the proposal and the messaging. Thanks 🙇 !
Proposed Changes
sudo-promptpackage to prompt the user for admin permission actions.Testing Instructions
Windows
No Optimizations
%appdata%\Studio.Speed Up Sites Optimization
%appdata%\Studio.Manual OptimizationRemove app data by deleting the folder located at%appdata%\Studio.Remove all created sites.Open the app.Observe that the onboarding screen is shown.Click on the Add Site button.Observe a dialog is displayed with three options.Click on the second option to display documentation.Observe the browser is opened and show documentation about how to speed up sites on Windows.Observe the site is created in the meantime.Prompt Only Displayed First Time
%appdata%\Studio.Help Section
macOS
No Dialog Displayed
$HOME/Library/Application Support/Studio.No New Help Option
Pre-merge Checklist