-
Notifications
You must be signed in to change notification settings - Fork 49
Add site context menu #1796
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
base: trunk
Are you sure you want to change the base?
Add site context menu #1796
Conversation
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.
Great work Nick! 🎉
Thanks for adding the site context menu.
I've pushed a commit fixing the tests. I'll take a closer look next week, but feature wise, everything worked smoothly.
One suggestion, what do you think if we leave open site and open wp-admin enabled for stopped sites too?
We could start the site before opening the browser.

@sejas, good call. I was just following the original design, but the other "Open site" and "WP admin" links in the app start up the site if it has not already been started. I updated the functionality in d1fa0db |
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.
@ndiego, thanks for applying my suggestion of starting the site to open the site or wp-admin. It works great.
I reviewed a second time and I think it's super close to be merged. I think it would be awesome to include it in v1.6.0.
I'll investigate a way to remove the setTimeout when opening the modal, and I'll create a PR with a slight modification around that.
I left a suggestion to remove unused parameters. The rest of the code looks and works great.
Could you add a new entry in the release notes?
Lines 1 to 4 in d1fa0db
Unreleased | |
========== | |
* Upgraded Playground dependencies to support Playground CLI and make offline site creation faster #1594 | |
* Added support for Sublime Text as a code editor #1723 |
src/components/site-menu.tsx
Outdated
? `${ site.name.substring( 0, MAX_LENGTH_SITE_TITLE - 3 ) }…` | ||
: site.name; | ||
|
||
const { response, checkboxChecked } = await ipcApi.showMessageBox( { |
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.
It seems to be repeating a bunch of code from delete-site.tsx
. Do you think we should extract it at this point?
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 extracted the common code into a new hook: bb8cf40
…#1807) * Remove setTimeout and move edit modal state to site-content-tabs * Move the edit modal is open state to use site details context * fix tests * fix tests
@ndiego, do you want to continue working on this PR, or would you prefer that I push a few changes? I think the main change left is unifying the delete site logic. |
Closing the loop here. Passing this off to you @sejas 🙏 |
📊 Performance Test ResultsComparing 7798813 vs trunk site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change |
Proposed Changes
This PR adds a context menu to sites in the sidebar, and was originally designed last year:
While a relatively low-priority feature, the context menu is a significant QOL improvement, especially for Studio power users. Simple actions like deleting a site now take three clicks rather than up to five clicks.
This PR does not add new functionality to Studio, such as the "Duplicate site..." option shown in the design mockup above. It simply adds easy access to all the existing functionality in Studio.
Here are the various states:
A couple of notes:
Here's a quick video of this feature in action:
studio-context-menu.mp4
Testing Instructions
Pre-merge Checklist