Merge pull request #7687 from weseek/feat/86710-121390-show-user-picture-tooltip

feat: Clean up client design

apps/app/src/components/ReactMarkdownComponents/Attachment.module.scss

@@ -0,0 +1,7 @@
+.attachment :global {
+  .attachment-icon {
+    flex-shrink: 0;
+    width: 35px;
+    height: 35px;
+  }
+}

apps/app/src/components/ReactMarkdownComponents/Attachment.tsx

@@ -6,6 +6,8 @@ import prettyBytes from 'pretty-bytes';
 import { useSWRxAttachment } from '~/stores/attachment';
 import { useDeleteAttachmentModal } from '~/stores/modal';
 
+import styles from './Attachment.module.scss';
+
 export const Attachment: React.FC<{
   attachmentId: string,
   url: string,
@@ -13,6 +15,7 @@ export const Attachment: React.FC<{
 }> = React.memo(({ attachmentId, url, attachmentName }) => {
   const { data: attachment, remove } = useSWRxAttachment(attachmentId);
   const { open: openDeleteAttachmentModal } = useDeleteAttachmentModal();
+
   const onClickTrashButtonHandler = useCallback(() => {
     if (attachment == null) {
       return;
@@ -21,40 +24,43 @@ export const Attachment: React.FC<{
   }, [attachment, openDeleteAttachmentModal, remove]);
 
   if (attachment == null) {
-    return (
-      <span className='text-muted'>This attachment not found.</span>
-    );
+    return <span className='text-muted'>The attachment could not be found.</span>;
   }
 
-  // TODO: locale support
-  // TODO: User Picture Tooltip
-  // TODO: Ensure that the card style does not collapse when d-inline-blocked
+  const {
+    filePathProxied,
+    originalName,
+    downloadPathProxied,
+    creator,
+    createdAt,
+    fileSize,
+  } = attachment;
 
   return (
-    <div className="card my-3">
-      <div className="card-body py-1">
-        <div className='row'>
-          <div className='col-1 px-0 d-flex align-items-center justify-content-center'>
-            <img src='/images/icons/editor/attachment.svg'/>
+    <div className={`${styles.attachment} d-inline-block`}>
+      <div className="my-2 p-2 card">
+        <div className="p-1 card-body d-flex align-items-center">
+          <div className='mr-2 px-0 d-flex align-items-center justify-content-center'>
+            <img src='/images/icons/editor/attachment.svg' className="attachment-icon" alt='attachment icon'/>
           </div>
-          <div className='col-11 pl-0'>
-            <div>
-              <a target="_blank" rel="noopener noreferrer" href={attachment.filePathProxied}>{attachment.originalName}</a>
-              <span className='ml-2'>
-                <a className="attachment-download" href={attachment.downloadPathProxied}>
-                  <i className="icon-cloud-download" />
-                </a>
-              </span>
-              <span className='ml-2'>
-                <a className="text-danger attachment-delete" onClick={onClickTrashButtonHandler}>
-                  <i className="icon-trash" />
-                </a>
-              </span>
+          <div className='pl-0'>
+            <div className='d-inline-block'>
+              <a target="_blank" rel="noopener noreferrer" href={filePathProxied}>
+                {attachmentName || originalName}
+              </a>
+              <a className="ml-2 attachment-download" href={downloadPathProxied}>
+                <i className="icon-cloud-download"/>
+              </a>
+              <a className="ml-2 text-danger attachment-delete" onClick={onClickTrashButtonHandler}>
+                <i className="icon-trash"/>
+              </a>
             </div>
-            <div>
-              <UserPicture user={attachment.creator} size="sm"></UserPicture>
-              <span className='ml-2 text-muted'>{new Date(attachment.createdAt).toLocaleString('ja-JP')}</span>
-              <span className='border-left ml-2 pl-2 text-muted'>{prettyBytes(attachment.fileSize)}</span>
+            <div className='d-flex align-items-center'>
+              <UserPicture user={creator} size="sm"/>
+              <span className='ml-2 text-muted'>
+                {new Date(createdAt).toLocaleString('en-US')}
+              </span>
+              <span className='ml-2 pl-2 border-left text-muted'>{prettyBytes(fileSize)}</span>
             </div>
           </div>
         </div>

apps/app/src/server/routes/apiv3/attachment.js

@@ -58,7 +58,11 @@ module.exports = (crowi) => {
     try {
       const attachmentId = req.query.attachmentId;
 
-      const attachment = await Attachment.findById(attachmentId);
+      const attachment = await Attachment.findById(attachmentId).populate('creator');
+
+      if (attachment.creator != null && attachment.creator instanceof User) {
+        attachment.creator = serializeUserSecurely(attachment.creator);
+      }
 
       return res.apiv3({ attachment });
     }

apps/app/src/services/renderer/remark-plugins/attachment.ts

@@ -1,5 +1,8 @@
+import path from 'path';
+
 import { Schema as SanitizeOption } from 'hast-util-sanitize';
 import { Plugin } from 'unified';
+import { Node } from 'unist';
 import { visit } from 'unist-util-visit';
 
 const SUPPORTED_ATTRIBUTES = ['attachmentId', 'url', 'attachmentName'];
@@ -10,29 +13,23 @@ const isAttachmentLink = (url: string) => {
   return url.match(attachmentUrlFormat);
 };
 
+const rewriteNode = (node: Node) => {
+  const attachmentId = path.basename(node.url as string);
+  const data = node.data ?? (node.data = {});
+  data.hName = 'attachment';
+  data.hProperties = {
+    attachmentId,
+    url: node.url,
+    attachmentName: (node.children as any)[0]?.value,
+  };
+};
+
 export const remarkPlugin: Plugin = () => {
   return (tree) => {
-    // TODO: do not use any for node.children[0].value
-    visit(tree, (node: any) => {
+    visit(tree, (node) => {
       if (node.type === 'link') {
-        if (isAttachmentLink(node.url)) {
-          const pathName = node.url.split('/');
-          const data = node.data ?? (node.data = {});
-          data.hName = 'attachment';
-          data.hProperties = {
-            attachmentId: pathName[2],
-            url: node.url,
-            attachmentName: node.children[0].value,
-          };
-
-          // omit position to fix the key regardless of its position
-          // see:
-          //   https://github.com/remarkjs/react-markdown/issues/703
-          //   https://github.com/remarkjs/react-markdown/issues/466
-          //
-          //   https://github.com/remarkjs/react-markdown/blob/a80dfdee2703d84ac2120d28b0e4998a5b417c85/lib/ast-to-react.js#L201-L204
-          //   https://github.com/remarkjs/react-markdown/blob/a80dfdee2703d84ac2120d28b0e4998a5b417c85/lib/ast-to-react.js#L217-L222
-          delete node.position;
+        if (isAttachmentLink(node.url as string)) {
+          rewriteNode(node);
         }
       }
     });

apps/app/src/stores/modal.tsx

@@ -700,6 +700,7 @@ export const useDeleteAttachmentModal = (): SWRResponse<DeleteAttachmentModalSta
     open,
     close,
   };
+};
 
 /*
  * LinkEditModal