Просмотр исходного кода

Merge branch 'feat/show-draw-attention-icon-for-broken-path' into feat/show-tooltip-for-rename-operation

yohei0125 3 лет назад
Родитель
Сommit
8c5280d881

+ 2 - 1
packages/app/resource/locales/en_US/translation.json

@@ -1118,6 +1118,7 @@
     }
   },
   "page_operation":{
-    "paths_recovered": "Paths recovered successfully"
+    "paths_recovered": "Paths recovered successfully",
+    "path_recovery_failed":"Path recovery failed"
   }
 }

+ 2 - 1
packages/app/resource/locales/ja_JP/translation.json

@@ -1111,6 +1111,7 @@
     }
   },
   "page_operation":{
-    "paths_recovered": "パスを修復しました"
+    "paths_recovered": "パスを修復しました",
+    "path_recovery_failed":"パスを修復できませんでした"
   }
 }

+ 2 - 1
packages/app/resource/locales/zh_CN/translation.json

@@ -1121,6 +1121,7 @@
     }
   },
   "page_operation":{
-    "paths_recovered": "成功恢复了页面路径"
+    "paths_recovered": "成功恢复了页面路径",
+    "path_recovery_failed":"路径恢复失败"
   }
 }

+ 1 - 6
packages/app/src/client/services/page-operation.ts

@@ -66,10 +66,5 @@ export const exportAsMarkdown = (pageId: string, revisionId: string, format: str
  * send request to fix broken paths caused by unexpected events such as server shutdown while renaming page paths
  */
 export const resumeRenameOperation = async(pageId: string): Promise<void> => {
-  try {
-    await apiv3Post('/pages/resume-rename', { pageId });
-  }
-  catch (err) {
-    throw err;
-  }
+  await apiv3Post('/pages/resume-rename', { pageId });
 };

+ 5 - 1
packages/app/src/components/Common/Dropdown/PageItemControl.tsx

@@ -133,6 +133,10 @@ const PageItemControlDropdownMenu = React.memo((props: DropdownMenuProps): JSX.E
     const showDeviderBeforeAdditionalMenuItems = (forceHideMenuItems?.length ?? 0) < 3;
     const showDeviderBeforeDelete = AdditionalMenuItems != null || showDeviderBeforeAdditionalMenuItems;
 
+    // PathRecovery
+    // Todo: It is wanted to find a better way to pass operationProcessData to PageItemControl
+    const shouldShowPathRecoveryButton = operationProcessData?.Rename != null ? operationProcessData?.Rename.isProcessable : false;
+
     contents = (
       <>
         { !isEnableActions && (
@@ -197,7 +201,7 @@ const PageItemControlDropdownMenu = React.memo((props: DropdownMenuProps): JSX.E
         ) }
 
         {/* PathRecovery */}
-        { !forceHideMenuItems?.includes(MenuItemType.PATH_RECOVERY) && isEnableActions && operationProcessData?.Rename != null && (
+        { !forceHideMenuItems?.includes(MenuItemType.PATH_RECOVERY) && isEnableActions && shouldShowPathRecoveryButton && (
           <DropdownItem
             onClick={pathRecoveryItemClickedHandler}
             className="grw-page-control-dropdown-item"

+ 16 - 31
packages/app/src/components/Sidebar/PageTree/Item.tsx

@@ -108,7 +108,6 @@ const Item: FC<ItemProps> = (props: ItemProps) => {
   const [isNewPageInputShown, setNewPageInputShown] = useState(false);
   const [shouldHide, setShouldHide] = useState(false);
   const [isRenameInputShown, setRenameInputShown] = useState(false);
-  const [isRenaming, setRenaming] = useState(false);
   const [isCreating, setCreating] = useState(false);
 
   const { data, mutate: mutateChildren } = useSWRxPageChildren(isOpen ? page._id : null);
@@ -270,7 +269,6 @@ const Item: FC<ItemProps> = (props: ItemProps) => {
 
     try {
       setRenameInputShown(false);
-      setRenaming(true);
       await apiv3Put('/pages/rename', {
         pageId: page._id,
         revisionId: page.revision,
@@ -285,14 +283,8 @@ const Item: FC<ItemProps> = (props: ItemProps) => {
     }
     catch (err) {
       setRenameInputShown(true);
-      setRenaming(false);
       toastError(err);
     }
-    finally {
-      setTimeout(() => {
-        setRenaming(false);
-      }, 1000);
-    }
   };
 
   const deleteMenuItemClickHandler = useCallback(async(_pageId: string, pageInfo: IPageInfoAll | undefined): Promise<void> => {
@@ -371,26 +363,22 @@ const Item: FC<ItemProps> = (props: ItemProps) => {
     return null;
   };
 
+  /**
+   * Users do not need to know if all pages have been renamed.
+   * Make resuming rename operation appears to be working fine to allow users for a seamless operation.
+   */
   const pathRecoveryMenuItemClickHandler = async(pageId: string): Promise<void> => {
     try {
-      setRenaming(true);
       await resumeRenameOperation(pageId);
 
-      mutateChildren();
-
       if (onRenamed != null) {
         onRenamed();
       }
 
       toastSuccess(t('page_operation.paths_recovered'));
     }
-    catch (err) {
-      toastError(err);
-    }
-    finally {
-      setTimeout(() => {
-        setRenaming(false);
-      }, 1000);
+    catch {
+      toastError(t('page_operation.path_recovery_failed'));
     }
   };
 
@@ -421,8 +409,8 @@ const Item: FC<ItemProps> = (props: ItemProps) => {
   }, [data, isOpen, targetPathOrId]);
 
   // Rename process
-  const existRenameProcessData = page.processData?.Rename != null;
-  const isRenameProcessable = page.processData?.Rename?.isProcessable;
+  // Icon that draw attention from users for some actions
+  const shouldShowAttentionIcon = page.processData?.Rename != null ? page.processData.Rename.isProcessable : false;
 
   return (
     <div
@@ -461,17 +449,13 @@ const Item: FC<ItemProps> = (props: ItemProps) => {
           )
           : (
             <>
-              { (isRenaming || (existRenameProcessData && !isRenameProcessable)) && (
-                <i className="fa fa-spinner fa-pulse mr-2 text-muted"></i>
-              )}
-              { (!isRenaming && isRenameProcessable) && (
-                <i id="path-recovery" className="fa fa-warning mr-2 text-warning"></i>
-              )}
-
-              {page.processData != null && (
-                <UncontrolledTooltip placement="top" target="path-recovery" fade={false}>
-                  {t('tooltip.operation.attention.rename')}
-                </UncontrolledTooltip>
+              { shouldShowAttentionIcon && (
+                <>
+                  <i id="path-recovery" className="fa fa-warning mr-2 text-warning"></i>
+                  <UncontrolledTooltip placement="top" target="path-recovery" fade={false}>
+                    {t('tooltip.operation.attention.rename')}
+                  </UncontrolledTooltip>
+                </>
               )}
 
               <a href={`/${page._id}`} className="grw-pagetree-title-anchor flex-grow-1">
@@ -494,6 +478,7 @@ const Item: FC<ItemProps> = (props: ItemProps) => {
             onClickDeleteMenuItem={deleteMenuItemClickHandler}
             onClickPathRecoveryMenuItem={pathRecoveryMenuItemClickHandler}
             isInstantRename
+            // Todo: It is wanted to find a better way to pass operationProcessData to PageItemControl
             operationProcessData={page.processData}
           >
             {/* pass the color property to reactstrap dropdownToggle props. https://6-4-0--reactstrap.netlify.app/components/dropdowns/  */}

+ 4 - 2
packages/app/src/server/routes/apiv3/pages.js

@@ -565,7 +565,9 @@ module.exports = (crowi) => {
     // The user has permission to resume rename operation if page is returned.
     const page = await Page.findByIdAndViewer(pageId, user, null, true);
     if (page == null) {
-      return res.apiv3Err(new ErrorV3('The operation is forbidden for this user.'), 403);
+      const msg = 'The operation is forbidden for this user';
+      const code = 'forbidden-user';
+      return res.apiv3Err(new ErrorV3(msg, code), 403);
     }
 
     try {
@@ -573,7 +575,7 @@ module.exports = (crowi) => {
     }
     catch (err) {
       logger.error(err);
-      return res.apiv3Err(new ErrorV3(`Failed to resume rename operation. ${err}`), 500);
+      return res.apiv3Err(err, 500);
     }
     return res.apiv3();
   });