Repository navigation
Add a right-click menu to sites in the sidebar - #686
amitraj2203 wants to merge 2 commits into
Conversation
A site in the sidebar offered no actions of its own: the only menu was in the page header, and that acted on whichever site was open, not the one being pointed at (WordPress#666). Right-clicking a site's row, or pressing Shift+F10 or the Menu key on it, now opens the same Site actions menu, scoped to that row's site and without changing which site is open. The menu's contents are shared with the header button through one SiteMenuItems rendering, so the two stay in sync automatically. The anchor position is resolved by a small pure function split into its own module, since index.jsx cannot be loaded without a DOM and nothing in node --test could otherwise reach it.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
|
@amitraj2203 is the ready for review? |
|
@ryanwelcher Yes |
zaerl
left a comment
zaerl
left a comment
There was a problem hiding this comment.
Thank you, this is great work.
@amitraj2203 @ryanwelcher I will only add a minor optimization here:
If a user right-clicks a sidebar entry different from the current one and selects "Update to latest trunk" the selected entry should change to that one in order to show the user the loading box in the main column.
Otherwise, it seems nothing is happening. What do you think?
Update to latest trunk's progress only renders on the active site, so choosing it from a different row's context menu ran the update with nothing visible for it (WordPress#666 review). That action now opens the site it targets first, while every other action in the menu keeps acting on the right-clicked site without opening it.
|
Thanks for the review! I have made the changes |
|
@ryanwelcher, can you please remove me as a reviewer from the PRs in this repository? I've removed myself as an admin. When you have time, thanks. |
Why
A site in the sidebar offers no actions of its own: the only menu is in the page header, and that acts on whichever site is open, not the one a contributor is pointing at. Users right-click a sidebar entry expecting the same menu the header offers (#666).
What changes
Right-clicking a site's row in the sidebar, or pressing Shift+F10 or the Menu key while it has focus, opens the same "Site actions" menu the page header already has, scoped to that row's site and without changing which site is open. The menu's contents are shared with the header button through one
SiteMenuItemsrendering, so the two offer the same actions in the same order and stay in sync automatically.How to test this
Starting state:
What must not have happened:
aria-pressedstate stay as they were.Covered by a new e2e journey,
tests/e2e/journeys/shell.spec.js:110, and a new unit test for the anchor-position arithmetic,tests/unit/context-menu-position.test.cjs.Risks and limitations
None known. No new IPC surface, no new dependency, no platform-specific gap.
Related
Fixes #666
Design decisions and alternatives considered
The context menu reuses the existing header menu's model (
siteMenuItems) and rendering (SiteMenuItems) rather than a second implementation, so the two can't drift out of sync. The anchor is a fixed, invisible trigger button positioned at the requested point, since the menu library needs a real trigger element to anchor and manage focus around; the actual control the contributor presses stays the site's row.The anchor-position arithmetic (a real pointer event's own point, or the pressed row's centre for a keyboard-triggered request) was split into its own pure module,
src/renderer/context-menu-position.cjs, becauseindex.jsxcannot be loaded without a DOM and nothing innode --testcould otherwise reach it.Review outcome (required — see AGENTS.md)
.agents/skills/self-review/SKILL.md), against.github/instructions/code-review.instructions.md; reviewed working-tree diff onfeature/666-sidebar-context-menuvs basetrunkat60078fe. 2 findings, both fixed: branching position logic moved out of the component intocontext-menu-position.cjswith its own unit test, and a bug where a real pointer event'sclientX/clientYof0was mistaken for a missing one (now told apart by the event'stypeinstead).Implementation notes
sites-sidebar.jsxkeys off.dataviews-view-list__item-wrapper, a class from@wordpress/dataviews's own list-layout source. It is undocumented as a public contract, but a future rename there just no-ops the context menu (theclosest()lookup fails silently) rather than throwing.