diff --git a/src/BloomBrowserUI/bookEdit/pageThumbnailList/pageThumbnailList.tsx b/src/BloomBrowserUI/bookEdit/pageThumbnailList/pageThumbnailList.tsx index 0f11c980b006..b0cfbad45d40 100644 --- a/src/BloomBrowserUI/bookEdit/pageThumbnailList/pageThumbnailList.tsx +++ b/src/BloomBrowserUI/bookEdit/pageThumbnailList/pageThumbnailList.tsx @@ -8,9 +8,10 @@ import $ from "jquery"; import { css } from "@emotion/react"; +import { Menu } from "@mui/material"; import * as React from "react"; -import { useState, useEffect, useMemo } from "react"; +import { useState, useEffect } from "react"; import * as ReactDOM from "react-dom"; import theOneLocalizationManager from "../../lib/localizationManager/localizationManager"; @@ -18,9 +19,16 @@ import * as toastr from "toastr"; import "errorHandler"; import WebSocketManager from "../../utils/WebSocketManager"; import { Responsive } from "react-grid-layout"; -import { get, postJson, postString, useApiData } from "../../utils/bloomApi"; +import { + get, + getAsync, + postJson, + postString, + useApiData, +} from "../../utils/bloomApi"; import { PageThumbnail } from "./PageThumbnail"; import LazyLoad, { forceCheck } from "react-lazyload"; +import { LocalizableMenuItem } from "../../react_components/localizableMenuItem"; // We're using the Responsive version of react-grid-layout because // (1) the previous version of the page thumbnails, which this replaces, @@ -57,6 +65,19 @@ export interface IPage { content: string; } +interface IPageMenuItem { + id: string; + label: string; + l10nId: string; + enabled?: boolean; +} + +interface IContextMenuPoint { + mouseX: number; + mouseY: number; + pageId: string; +} + // This map goes from page ID to a callback that we get from the page thumbnail // which should be called when the main Bloom program informs us that // the thumbnail needs to be updated. @@ -75,6 +96,11 @@ const PageList: React.FunctionComponent<{ pageLayout: string }> = (props) => { // when the websocket detects a request for this. const [resetValue, setResetValue] = useState(1); const [twoColumns, setTwoColumns] = useState(true); + const [contextMenuPoint, setContextMenuPoint] = + useState(); + const [contextMenuItems, setContextMenuItems] = useState( + [], + ); const [selectedPageId, setSelectedPageId] = useState(""); const bookAttributesThatMayAffectDisplay = useApiData( @@ -82,6 +108,31 @@ const PageList: React.FunctionComponent<{ pageLayout: string }> = (props) => { {}, ); + const pageMenuDefinition: IPageMenuItem[] = [ + { + id: "duplicatePage", + label: "Duplicate Page", + l10nId: "EditTab.DuplicatePageButton", + }, + { + id: "duplicatePageManyTimes", + label: "Duplicate Page Many Times...", + l10nId: "EditTab.DuplicatePageMultiple", + }, + { id: "copyPage", label: "Copy Page", l10nId: "EditTab.CopyPage" }, + { id: "pastePage", label: "Paste Page", l10nId: "EditTab.PastePage" }, + { + id: "removePage", + label: "Remove Page", + l10nId: "EditTab.DeletePageButton", + }, + { + id: "chooseDifferentLayout", + label: "Choose Different Layout", + l10nId: "EditTab.ChooseLayoutButton", + }, + ]; + // All the code in this useEffect is one-time initialization. useEffect(() => { let localizedNotification = ""; @@ -198,13 +249,13 @@ const PageList: React.FunctionComponent<{ pageLayout: string }> = (props) => { // https://stackoverflow.com/questions/61191496/why-is-my-react-lazyload-component-not-working) // This actually runs each time a page is deleted. forceCheck(); - }, [realPageList]); + }, [realPageList, selectedPageId]); // this is embedded so that we have access to realPageList const handleGridItemClick = (e: React.MouseEvent) => { e.stopPropagation(); e.preventDefault(); - NotifyCSharpOfClick(); + closeContextMenu(); // for manual testing if (e.getModifierState("Control") && e.getModifierState("Alt")) { @@ -230,17 +281,70 @@ const PageList: React.FunctionComponent<{ pageLayout: string }> = (props) => { } }; + const openContextMenuCount = React.useRef(0); + + const closeContextMenu = () => { + openContextMenuCount.current = 0; + setContextMenuPoint(undefined); + setContextMenuItems([]); + }; + + const openContextMenu = (pageId: string, x: number, y: number) => { + openContextMenuCount.current++; + const currentCount = openContextMenuCount.current; + // If the user right-clicks on a page that is not selected, ignore the click and + // close any existing context menu. + if (pageId !== selectedPageId) { + closeContextMenu(); + return; + } + + Promise.all( + pageMenuDefinition.map(async (item) => { + const response = await getAsync( + `pageList/contextMenuItemEnabled?commandId=${encodeURIComponent(item.id)}&pageId=${encodeURIComponent(pageId)}`, + ); + return { + id: item.id, + label: item.label, + l10nId: item.l10nId, + enabled: !!response.data, + }; + }), + ).then((menuItems) => { + if (currentCount !== openContextMenuCount.current) { + // Either the menu was closed or a newer context menu has been opened since this async + // call was made, so ignore the results. + return; + } + setContextMenuItems(menuItems); + setContextMenuPoint({ + mouseX: x - 2, + mouseY: y - 4, + pageId, + }); + }); + }; + + const onContextMenuItemClick = (commandId: string) => { + if (!contextMenuPoint) return; + + postJson("pageList/contextMenuItemClicked", { + pageId: contextMenuPoint.pageId, + commandId, + }); + closeContextMenu(); + }; + const handleContextMenu = (e: React.MouseEvent) => { e.stopPropagation(); e.preventDefault(); - NotifyCSharpOfClick(); if (e.currentTarget) { const pageElt = e.currentTarget.closest("[id]")!; const pageId = pageElt.getAttribute("id"); - if (pageId === selectedPageId) - postJson("pageList/menuClicked", { - pageId, - }); + if (pageId) { + openContextMenu(pageId, e.clientX, e.clientY); + } } }; @@ -255,66 +359,63 @@ const PageList: React.FunctionComponent<{ pageLayout: string }> = (props) => { }, ...realPageList, ]; - const pages = useMemo(() => { - const pages1 = pageList.map((pageContent, index) => { - return ( -
{ + return ( +
+ - - - pageIdToRefreshMap.set(id, callback) - } - onClick={handleGridItemClick} - onContextMenu={handleContextMenu} - /> - {selectedPageId === pageContent.key && ( - - )} - -
- ); - }); - return pages1; - }, [pageList]); + + pageIdToRefreshMap.set(id, callback) + } + onClick={handleGridItemClick} + onContextMenu={handleContextMenu} + /> + {selectedPageId === pageContent.key && ( + + )} + +
+ ); + }); // Set up some objects and functions we need as params for our main element. // Some of them come in sets "lg" and "sm". Currently the "lg" (two-column) @@ -391,6 +492,42 @@ const PageList: React.FunctionComponent<{ pageLayout: string }> = (props) => { > {pages} + {contextMenuPoint && ( + + {contextMenuItems.map((item) => ( + onContextMenuItemClick(item.id)} + dontGiveAffordanceForCheckbox={true} + css={css` + display: block !important; + min-height: 24px !important; + padding: 3px 8px !important; + `} + labelCss={css` + font-size: 10pt !important; + white-space: normal !important; + line-height: normal !important; + `} + > + {item.label} + + ))} + + )} ); }; @@ -402,20 +539,8 @@ $(window).ready(() => { , document.getElementById("pageGridWrapper"), ); - - // If the user clicks outside of the context menu, we want to close it. - // Since it is currently a winforms menu, we do that by sending a message - // back to c#-land. - // We can remove this in 5.6, or whenever we replace the winforms context menu with a rect menu. - $(window).click(() => { - NotifyCSharpOfClick(); - }); }); -function NotifyCSharpOfClick() { - (window as any).chrome?.webview?.postMessage("browser-clicked"); -} - // Function invoked when dragging a page ends. Note that it is often // called when all the user intended was to click the page, presumably // because some tiny movement was made while the mouse is down. diff --git a/src/BloomBrowserUI/react_components/localizableMenuItem.tsx b/src/BloomBrowserUI/react_components/localizableMenuItem.tsx index bef032021309..4b34e181cc40 100644 --- a/src/BloomBrowserUI/react_components/localizableMenuItem.tsx +++ b/src/BloomBrowserUI/react_components/localizableMenuItem.tsx @@ -1,4 +1,4 @@ -import { css } from "@emotion/react"; +import { css, SerializedStyles } from "@emotion/react"; import * as React from "react"; import { Fragment, ReactNode, useEffect, useState } from "react"; @@ -62,6 +62,15 @@ export interface ILocalizableMenuItemProps subscriptionTooltipOverride?: string; className?: string; isDivider?: boolean; + // The optional css`` for the label text element, usually an h6 inside the li. + // Typical settings could be font-size and line-height to control the size of + // the menu text, and white-space to allow wrapping of the item's text. + // + // css for the entire MenuItem can be passed in what would be props.css, but only + // if props.className is NOT being used. Typical settings for that would be padding + // and min-height to control the spacing of the menu item, and display:block to + // override MUI's inline-flex and allow the item to size vertically to its content. + labelCss?: SerializedStyles; } interface ILocalizableCheckboxMenuItemProps @@ -178,6 +187,7 @@ export const LocalizableMenuItem: React.FunctionComponent< font-weight: 400 !important; // H6 defaults to 500; too thick font-family: ${kUiFontStack}; color: ${menuItemColor} !important; + ${props.labelCss ? props.labelCss : css``} // We can't use the disabled prop because it prevents the click from opening settings and // prevents the tooltip. So we just make it look disabled (using the same setting as Mui-disabled). diff --git a/src/BloomExe/Edit/PageListController.cs b/src/BloomExe/Edit/PageListController.cs index c50d856b9b17..9b2be121eb49 100644 --- a/src/BloomExe/Edit/PageListController.cs +++ b/src/BloomExe/Edit/PageListController.cs @@ -5,7 +5,6 @@ using Bloom.Api; using Bloom.Book; using Bloom.web; -using L10NSharp; using SIL.Reporting; using SIL.Windows.Forms.Reporting; @@ -35,92 +34,6 @@ BloomWebSocketServer webSocketServer _thumbNailList.RelocatePageEvent = relocatePageEvent; _thumbNailList.PageSelectedChanged += new EventHandler(OnPageSelectedChanged); _thumbNailList.Model = model; - // First action determines whether the menu item is enabled, second performs it. - var menuItems = new List(); - menuItems.Add( - new PageThumbnailList.MenuItemSpec() - { - Label = LocalizationManager.GetString( - "EditTab.DuplicatePageButton", - "Duplicate Page" - ), - EnableFunction = (page) => page != null && _model.CanDuplicatePage, - ExecuteCommand = (page) => _model.DuplicatePage(page), - } - ); - menuItems.Add( - new PageThumbnailList.MenuItemSpec() - { - Label = LocalizationManager.GetString( - "EditTab.DuplicatePageMultiple", - "Duplicate Page Many Times..." - ), - EnableFunction = (page) => page != null && _model.CanDuplicatePage, - ExecuteCommand = (page) => _model.DuplicateManyPages(page), - } - ); - menuItems.Add( - new PageThumbnailList.MenuItemSpec() - { - Label = LocalizationManager.GetString("EditTab.CopyPage", "Copy Page"), - EnableFunction = (page) => page != null && _model.CanCopyPage, - ExecuteCommand = (page) => _model.CopyPage(page), - } - ); - menuItems.Add( - new PageThumbnailList.MenuItemSpec() - { - Label = LocalizationManager.GetString("EditTab.PastePage", "Paste Page"), - EnableFunction = (page) => - page != null && _model.CanAddPages && _model.GetClipboardHasPage(), - ExecuteCommand = (page) => _model.PastePage(page), - } - ); - menuItems.Add( - new PageThumbnailList.MenuItemSpec() - { - Label = LocalizationManager.GetString( - "EditTab.DeletePageButton", - "Remove Page" - ), - EnableFunction = (page) => page != null && _model.CanDeletePage, - ExecuteCommand = (page) => - { - if (ConfirmRemovePageDialog.Confirm()) - _model.DeletePage(page); - }, - } - ); - menuItems.Add( - new PageThumbnailList.MenuItemSpec() - { - Label = LocalizationManager.GetString( - "EditTab.ChooseLayoutButton", - "Choose Different Layout" - ), - // We don't want to allow layout changes for game pages (except Widget pages). - // Note: we also have to disable the Change Layout switch, in origami.ts - EnableFunction = (page) => - page != null - && !page.Required - && !page.GetDivNodeForThisPage() - .GetAttribute("data-tool-id") - .Equals("game"), - ExecuteCommand = (page) => - { - // While we have separate browsers running for this page list and the editing view, we switch - // the focus to the editing browser before launching the dialog so that Esc will work to close - // the dialog without interacting with the dialog first. - _model.GetEditingBrowser().Focus(); - _model.ChangePageLayout(page); - }, - } - ); - // This sets up the context menu items that will be shown when the user clicks the - // arrow in the thumbnail list or right-clicks in the page list. - // Note that we can't use ContextMenuProvider here, because there is no reasonable - // way to know which page was right-clicked, if any. So we handle right-click in JS. - _thumbNailList.ContextMenuItems = menuItems; } private void OnPageSelectedChanged(object page, EventArgs e) diff --git a/src/BloomExe/Edit/PageThumbnailList.cs b/src/BloomExe/Edit/PageThumbnailList.cs index fd9a9b042ce2..2b2f0a63fa08 100644 --- a/src/BloomExe/Edit/PageThumbnailList.cs +++ b/src/BloomExe/Edit/PageThumbnailList.cs @@ -50,17 +50,6 @@ internal PageListApi PageListApi internal BloomWebSocketServer WebSocketServer { get; set; } - internal class MenuItemSpec - { - public string Label; - public Func EnableFunction; // called to determine whether the item should be enabled. - public Action ExecuteCommand; // called when the item is chosen to perform the action. - } - - // A list of menu items that should be in both the web browser'movedPageIdAndNewIndex right-click menu and - // the one we show ourselves when the arrow is clicked. - internal List ContextMenuItems { get; set; } - public PageThumbnailList() { // set the thumbnail interval based on physical RAM @@ -207,40 +196,73 @@ internal void PageClicked(IPage page) InvokePageSelectedChanged(page); } - internal void MenuClicked(IPage page) + /// + /// Check whether or not the page thumbnail context menu item is enabled for the + /// given page. + /// + /// + /// The list of commandId values is found in pageThumbnailList.tsx in the + /// pageMenuDefinition array. + /// + internal bool IsContextMenuCommandEnabled(IPage page, string commandId) { if (!Enabled || page == null) - return; - var menu = new ContextMenuStrip(); - foreach (var item in ContextMenuItems) + return false; + + switch (commandId) { - var useItem = item; // for use in Click action (reference to loop variable has unpredictable results) - var menuItem = new ToolStripMenuItem(item.Label); - menuItem.Click += (sender, args) => useItem.ExecuteCommand(page); - menuItem.Enabled = item.EnableFunction(page); - menu.Items.Add(menuItem); + case "duplicatePage": + case "duplicatePageManyTimes": + return Model.CanDuplicatePage; + case "copyPage": + return Model.CanCopyPage; + case "pastePage": + return Model.CanAddPages && Model.GetClipboardHasPage(); + case "removePage": + return Model.CanDeletePage; + case "chooseDifferentLayout": + return !page.Required + && !page.GetDivNodeForThisPage() + .GetAttribute("data-tool-id") + .Equals("game"); + default: + return false; } - - Model.GetEditingBrowser().OnBrowserClick += Browser_Click; - _popupPageMenu = menu; - - menu.Show(Control.MousePosition); } - ContextMenuStrip _popupPageMenu; - private PageListApi _pageListApi; - internal bool Enabled = true; - - private void Browser_Click(object sender, EventArgs e) + internal void ExecuteContextMenuCommand(IPage page, string commandId) { - if (_popupPageMenu != null) + if (!IsContextMenuCommandEnabled(page, commandId)) + return; + + switch (commandId) { - _popupPageMenu.Close(ToolStripDropDownCloseReason.CloseCalled); - _popupPageMenu = null; - Model.GetEditingBrowser().OnBrowserClick -= Browser_Click; + case "duplicatePage": + Model.DuplicatePage(page); + break; + case "duplicatePageManyTimes": + Model.DuplicateManyPages(page); + break; + case "copyPage": + Model.CopyPage(page); + break; + case "pastePage": + Model.PastePage(page); + break; + case "removePage": + if (ConfirmRemovePageDialog.Confirm()) + Model.DeletePage(page); + break; + case "chooseDifferentLayout": + Model.GetEditingBrowser().Focus(); + Model.ChangePageLayout(page); + break; } } + private PageListApi _pageListApi; + internal bool Enabled = true; + // This gets invoked by Javascript (via the PageListApi) when it determines that a particular page has been moved. // newIndex is the (zero-based) index that the page is moving to // in the whole list of pages, including the placeholder. diff --git a/src/BloomExe/web/PageListApi.cs b/src/BloomExe/web/PageListApi.cs index 064f93078ef7..9bb66bf6f35a 100644 --- a/src/BloomExe/web/PageListApi.cs +++ b/src/BloomExe/web/PageListApi.cs @@ -3,6 +3,7 @@ using System.Diagnostics; using System.Dynamic; using System.Linq; +using System.Threading.Tasks; using System.Windows.Forms; using Bloom.Api; using Bloom.Book; @@ -10,6 +11,7 @@ using Bloom.Edit; using Bloom.SafeXml; using Bloom.Utils; +using SIL.Reporting; namespace Bloom.web { @@ -52,7 +54,19 @@ public void RegisterWithApiHandler(BloomApiHandler apiHandler) true ); apiHandler - .RegisterEndpointHandler("pageList/menuClicked", HandleShowMenuRequest, true) + .RegisterEndpointHandler( + "pageList/contextMenuItemEnabled", + HandleContextMenuItemEnabled, + false, + false + ) + .Measureable(); + apiHandler + .RegisterEndpointHandler( + "pageList/contextMenuItemClicked", + HandleContextMenuItemClickedRequest, + true + ) .Measureable(); apiHandler.RegisterEndpointHandler( @@ -109,14 +123,58 @@ private void HandlePageClickedRequest(ApiRequest request) request.PostSucceeded(); } - // User clicked on the down arrow, we respond by showing the same menu as for right click. - private void HandleShowMenuRequest(ApiRequest request) + private void HandleContextMenuItemEnabled(ApiRequest request) + { + var commandId = request.Parameters["commandId"]; + var pageId = request.Parameters["pageId"]; + IPage page = PageFromId(pageId); + if (page != null) + request.ReplyWithBoolean(PageList.IsContextMenuCommandEnabled(page, commandId)); + else + request.ReplyWithBoolean(false); + } + + private void HandleContextMenuItemClickedRequest(ApiRequest request) { var requestData = DynamicJson.Parse(request.RequiredPostJson()); string pageId = requestData.pageId; + string commandId = requestData.commandId; IPage page = PageFromId(pageId); + if (page != null) - PageList.MenuClicked(page); + { + // Execute the command asynchronously after a short delay + // The discard operator _ indicates we're intentionally not awaiting this + _ = Task.Run(async () => + { + await Task.Delay(100); // 100ms delay to let the UI respond + + // Execute on the UI thread using the form's synchronization context + var form = Shell.GetShellOrOtherOpenForm(); + if (form != null && !form.IsDisposed) + { + form.BeginInvoke( + new Action(() => + { + try + { + PageList.ExecuteContextMenuCommand(page, commandId); + } + catch (Exception ex) + { + // Log the error. Should we notify the user as well? + Logger.WriteEvent( + $"Error executing content menu command for {commandId} on page {pageId}" + ); + Logger.WriteError(ex); + } + }) + ); + } + }); + } + + // Return success immediately without waiting for the command to execute request.PostSucceeded(); }