address code review

chanzuckerberg · Aug 10, 2024 · 82e2951 · 82e2951
1 parent 1d62e39
commit 82e2951
Show file tree

Hide file tree

Showing 9 changed files with 41 additions and 48 deletions.
diff --git a/frontend/packages/data-portal/app/components/BrowseData/DepositionTable.tsx b/frontend/packages/data-portal/app/components/BrowseData/DepositionTable.tsx
@@ -18,7 +18,6 @@ import { DepositionTableWidths } from 'app/constants/table'
 import { Deposition, useDepositions } from 'app/hooks/useDepositions'
 import { useI18n } from 'app/hooks/useI18n'
 import { useIsLoading } from 'app/hooks/useIsLoading'
-import { I18nKeys } from 'app/types/i18n'
 import { LogLevel } from 'app/types/logging'
 import { cnsNoMerge } from 'app/utils/cns'
 import { sendLogs } from 'app/utils/logging'
@@ -295,7 +294,7 @@ export function DepositionTable() {
                           className="whitespace-nowrap overflow-x-hidden overflow-ellipsis"
                           key={entry[0]}
                         >
-                          {t(entry[1] as I18nKeys)}
+                          {t(entry[1])}
                         </li>
                       ))}
                   </ul>

diff --git a/frontend/packages/data-portal/app/components/CollapsibleList.tsx b/frontend/packages/data-portal/app/components/CollapsibleList.tsx
@@ -33,8 +33,7 @@ export function CollapsibleList({
         'flex flex-col gap-sds-xs',
         tableVariant
           ? 'text-sds-body-s leading-sds-body-s'
-          : 'text-sds-body-xxs leading-sds-body-xxs',
-        !tableVariant && 'text-sds-gray-600',
+          : 'text-sds-body-xxs leading-sds-body-xxs text-sds-gray-600',
         collapsible && 'transition-[max-height_0.2s_ease-out]',
       )}
     >

diff --git a/frontend/packages/data-portal/app/components/Dataset/DatasetMetadataTable.tsx b/frontend/packages/data-portal/app/components/Dataset/DatasetMetadataTable.tsx
@@ -14,17 +14,17 @@ import { DatasetType } from './type'
 
 export function DatasetMetadataTable({
   dataset,
-  allFields,
+  showAllFields,
   initialOpen,
 }: {
   dataset: DatasetType
-  allFields?: boolean
+  showAllFields?: boolean
   initialOpen?: boolean
 }) {
   const { t } = useI18n()
 
   const datasetMetadata = getTableData(
-    !!allFields && {
+    !!showAllFields && {
       label: t('datasetTitle'),
       values: [dataset.title ?? ''],
       renderValue: (value) => {
@@ -46,12 +46,12 @@ export function DatasetMetadataTable({
       },
     },
 
-    !!allFields && {
+    !!showAllFields && {
       label: t('datasetId'),
       values: [`${dataset.id ?? ''}`],
     },
 
-    !!allFields && {
+    !!showAllFields && {
       label: t('description'),
       values: [dataset.description ?? ''],
       className: 'text-ellipsis line-clamp-3',
@@ -62,17 +62,17 @@ export function DatasetMetadataTable({
       values: [dataset.deposition_date ?? ''],
     },
 
-    !!allFields && {
+    !!showAllFields && {
       label: t('releaseDate'),
       values: [dataset.release_date ?? ''],
     },
 
-    !!allFields && {
+    !!showAllFields && {
       label: t('lastModifiedDate'),
       values: [dataset.last_modified_date ?? ''],
     },
 
-    !!allFields && {
+    !!showAllFields && {
       label:
         dataset.authors && dataset.authors.length === 1
           ? t('author')
@@ -112,7 +112,7 @@ export function DatasetMetadataTable({
       },
     },
 
-    !!allFields && {
+    !!showAllFields && {
       label: t('publications'),
       values: [dataset.dataset_publications ?? ''],
       renderValue: (value: string) => {
@@ -124,7 +124,7 @@ export function DatasetMetadataTable({
   return (
     <AccordionMetadataTable
       id="dataset-metadata"
-      header={allFields ? t('dataset') : t('datasetMetadata')}
+      header={showAllFields ? t('dataset') : t('datasetMetadata')}
       data={datasetMetadata}
       initialOpen={initialOpen}
     />

diff --git a/frontend/packages/data-portal/app/components/Deposition/AnnotationsSummaryMetadataTable.tsx b/frontend/packages/data-portal/app/components/Deposition/AnnotationsSummaryMetadataTable.tsx
@@ -2,7 +2,6 @@ import { AccordionMetadataTable } from 'app/components/AccordionMetadataTable'
 import { CollapsibleList } from 'app/components/CollapsibleList'
 import { shapeTypeToI18nKeyPlural } from 'app/constants/objectShapeTypes'
 import { useI18n } from 'app/hooks/useI18n'
-import { I18nKeys } from 'app/types/i18n'
 import { getTableData } from 'app/utils/table'
 
 export function AnnotationsSummaryMetadataTable({
@@ -92,7 +91,7 @@ export function AnnotationsSummaryMetadataTable({
             {Object.entries(shapeTypeToI18nKeyPlural)
               .filter(([k]) => shapeTypes.includes(k))
               .map(([k, v]) => (
-                <li key={k}>{t(v as I18nKeys)}</li>
+                <li key={k}>{t(v)}</li>
               ))}
           </ul>
         )

diff --git a/frontend/packages/data-portal/app/components/Deposition/DepositionMetadataTable.tsx b/frontend/packages/data-portal/app/components/Deposition/DepositionMetadataTable.tsx
@@ -12,37 +12,23 @@ import { getTableData } from 'app/utils/table'
 
 function CollapsibleDescription({ text }: { text: string }) {
   const { t } = useI18n()
-  const [collapsed, setCollapsed] = useState(true)
+  const [isCollapsed, setCollapsed] = useState(true)
 
   return (
     <div>
-      <p className={collapsed ? 'text-ellipsis line-clamp-3' : undefined}>
+      <p className={isCollapsed ? 'text-ellipsis line-clamp-3' : undefined}>
         {text}
       </p>
       <div className="mt-sds-s font-semibold text-sds-primary-400">
-        <button type="button" onClick={() => setCollapsed(!collapsed)}>
+        <button type="button" onClick={() => setCollapsed((prev) => !prev)}>
           <span className="flex flex-row gap-sds-xxs items-center">
-            {collapsed ? (
-              <>
-                <Icon
-                  sdsIcon="plus"
-                  sdsSize="xs"
-                  sdsType="static"
-                  className="!text-current"
-                />
-                {t('showMore')}
-              </>
-            ) : (
-              <>
-                <Icon
-                  sdsIcon="minus"
-                  sdsSize="xs"
-                  sdsType="static"
-                  className="!text-current"
-                />
-                {t('showLess')}
-              </>
-            )}
+            <Icon
+              sdsIcon={isCollapsed ? 'plus' : 'minus'}
+              sdsSize="xs"
+              sdsType="static"
+              className="!text-current"
+            />
+            {t(isCollapsed ? 'showMore' : 'showLess')}
           </span>
         </button>
       </div>

diff --git a/...d/packages/data-portal/app/components/Deposition/MethodLinks/MethodLinksMetadataTable.tsx b/...d/packages/data-portal/app/components/Deposition/MethodLinks/MethodLinksMetadataTable.tsx
@@ -11,7 +11,7 @@ import { generateMethodLinks, MethodLinkVariantProps } from './common'
 
 const COLUMN_WIDTH = 170
 
-type MethodData = {
+type MethodDataType = {
   method_type: MethodType
   annotations_count: number
   method_description: string
@@ -65,7 +65,7 @@ function MethodSummarySection({
   data,
 }: {
   label?: string
-  data: MethodData
+  data: MethodDataType
 }) {
   const { t } = useI18n()
 
@@ -125,7 +125,7 @@ export function MethodLinksMetadataTable({
 }) {
   const { t } = useI18n()
 
-  const methodOne: MethodData = {
+  const methodOne: MethodDataType = {
     method_type: 'hybrid',
     annotations_count: 3000,
     method_description:
@@ -163,7 +163,7 @@ export function MethodLinksMetadataTable({
     ],
   }
 
-  const methodTwo: MethodData = {
+  const methodTwo: MethodDataType = {
     method_type: 'manual',
     annotations_count: 1000,
     method_description:
@@ -172,7 +172,7 @@ export function MethodLinksMetadataTable({
     links: [],
   }
 
-  const methodThree: MethodData = {
+  const methodThree: MethodDataType = {
     method_type: 'automated',
     annotations_count: 6000,
     method_description:

diff --git a/frontend/packages/data-portal/app/components/Run/RunMetadataDrawer.tsx b/frontend/packages/data-portal/app/components/Run/RunMetadataDrawer.tsx
@@ -18,7 +18,7 @@ export function RunMetadataDrawer() {
       label={i18n.runDetails}
     >
       <DatasetMetadataTable
-        allFields
+        showAllFields
         dataset={run.dataset}
         initialOpen={false}
       />

diff --git a/frontend/packages/data-portal/app/constants/objectShapeTypes.ts b/frontend/packages/data-portal/app/constants/objectShapeTypes.ts
@@ -1,11 +1,16 @@
-export const shapeTypeToI18nKey = {
+import { I18nKeys } from 'app/types/i18n'
+import { ObjectShapeType } from 'app/types/shapeTypes'
+
+type ShapeTypeToI18nKeyMap = { [key in ObjectShapeType]: I18nKeys }
+
+export const shapeTypeToI18nKey: ShapeTypeToI18nKeyMap = {
   InstanceSegmentation: 'instanceSegmentation',
   OrientedPoint: 'orientedPoint',
   Point: 'point',
   SegmentationMask: 'segmentationMask',
 }
 
-export const shapeTypeToI18nKeyPlural = {
+export const shapeTypeToI18nKeyPlural: ShapeTypeToI18nKeyMap = {
   InstanceSegmentation: 'instanceSegmentations',
   OrientedPoint: 'orientedPoints',
   Point: 'points',

diff --git a/frontend/packages/data-portal/app/types/shapeTypes.ts b/frontend/packages/data-portal/app/types/shapeTypes.ts
@@ -0,0 +1,5 @@
+export type ObjectShapeType =
+  | 'InstanceSegmentation'
+  | 'OrientedPoint'
+  | 'Point'
+  | 'SegmentationMask'