Browse Source

Code Review Fixes

Francesco Baccetti 4 years ago
parent
commit
0c2956dd57

+ 0 - 2
packages/app/package.json

@@ -49,8 +49,6 @@
     "@types/reach__router": "^1.3.5",
     "@types/react": "^16.9.0",
     "@types/react-dom": "^16.9.0",
-    "@types/react-redux": "^7.1.9",
-    "@types/redux": "^3.6.0",
     "@types/video.js": "^7.3.10",
     "apollo": "^2.30.2",
     "chromatic": "^4.0.3",

+ 1 - 2
packages/app/src/components/VideoGrid.tsx

@@ -3,7 +3,6 @@ import styled from '@emotion/styled'
 import { navigate } from '@reach/router'
 
 import routes from '@/config/routes'
-import { spacing } from '@/shared/theme'
 import { VideoFields } from '@/api/queries/__generated__/VideoFields'
 import { VideoPreview, Grid } from '@/shared/components'
 
@@ -16,7 +15,7 @@ type VideoGridProps = {
 }
 const VideoGrid: React.FC<VideoGridProps> = ({ videos }) => {
   return (
-    <Grid gap={spacing.xl}>
+    <Grid>
       {videos.map((v, idx) => (
         <StyledVideoPreview
           key={idx}

+ 3 - 2
packages/app/src/shared/components/Grid/Grid.tsx

@@ -1,6 +1,7 @@
 import React from 'react'
 import styled from '@emotion/styled'
 import useResizeObserver from 'use-resize-observer'
+import { spacing } from '../../theme'
 
 const Container = styled.div<GridProps>`
   display: grid;
@@ -14,7 +15,7 @@ type GridProps = {
   className?: string
   onResize?: (sizes: number[]) => void
 }
-const Grid: React.FC<GridProps> = ({ children, className, repeat = 'fit', onResize, ...props }) => {
+const Grid: React.FC<GridProps> = ({ children, className, gap = spacing.xl, repeat = 'fit', onResize, ...props }) => {
   const { ref: gridRef } = useResizeObserver<HTMLDivElement>({
     onResize: () => {
       if (onResize && gridRef.current) {
@@ -26,7 +27,7 @@ const Grid: React.FC<GridProps> = ({ children, className, repeat = 'fit', onResi
   })
 
   return (
-    <Container {...props} repeat={repeat} className={className} ref={gridRef}>
+    <Container {...props} repeat={repeat} className={className} ref={gridRef} gap={gap}>
       {children}
     </Container>
   )

+ 3 - 5
packages/app/src/shared/components/InfiniteVideoGrid/InfiniteVideoGrid.tsx

@@ -16,7 +16,7 @@ type InfiniteVideoGridProps = {
 }
 
 export const INITIAL_ROWS = 4
-export const VIDEOS_PER_ROW = 4
+export const INITIAL_VIDEOS_PER_ROW = 4
 
 const InfiniteVideoGrid: React.FC<InfiniteVideoGridProps> = ({
   title,
@@ -26,7 +26,7 @@ const InfiniteVideoGrid: React.FC<InfiniteVideoGridProps> = ({
   initialLoading,
   className,
 }) => {
-  const [videosPerRow, setVideosPerRow] = useState(VIDEOS_PER_ROW)
+  const [videosPerRow, setVideosPerRow] = useState(INITIAL_VIDEOS_PER_ROW)
 
   const loadedVideosCount = videos?.length || 0
   const videoRowsCount = Math.floor(loadedVideosCount / videosPerRow)
@@ -91,9 +91,7 @@ const InfiniteVideoGrid: React.FC<InfiniteVideoGridProps> = ({
   return (
     <section className={className}>
       {title && <Title>{title}</Title>}
-      <Grid gap={spacing.xl} onResize={(sizes) => setVideosPerRow(sizes.length)}>
-        {gridContent}
-      </Grid>
+      <Grid onResize={(sizes) => setVideosPerRow(sizes.length)}>{gridContent}</Grid>
     </section>
   )
 }

+ 2 - 2
packages/app/src/shared/components/InfiniteVideoGrid/index.ts

@@ -1,2 +1,2 @@
-import InfiniteVideoGrid, { INITIAL_ROWS, VIDEOS_PER_ROW } from './InfiniteVideoGrid'
-export { InfiniteVideoGrid as default, INITIAL_ROWS, VIDEOS_PER_ROW }
+import InfiniteVideoGrid, { INITIAL_ROWS, INITIAL_VIDEOS_PER_ROW } from './InfiniteVideoGrid'
+export { InfiniteVideoGrid as default, INITIAL_ROWS, INITIAL_VIDEOS_PER_ROW }

+ 1 - 1
packages/app/src/shared/components/index.ts

@@ -22,7 +22,7 @@ export { default as Sidenav, SIDENAV_WIDTH, EXPANDED_SIDENAV_WIDTH, NavItem } fr
 export { default as ChannelAvatar } from './ChannelAvatar'
 export { default as GlobalStyle } from './GlobalStyle'
 export { default as Placeholder } from './Placeholder'
-export { default as InfiniteVideoGrid, INITIAL_ROWS, VIDEOS_PER_ROW } from './InfiniteVideoGrid'
+export { default as InfiniteVideoGrid, INITIAL_ROWS, INITIAL_VIDEOS_PER_ROW } from './InfiniteVideoGrid'
 export { default as ToggleButton } from './ToggleButton'
 export { default as Icon } from './Icon'
 export { default as Searchbar } from './Searchbar'

+ 8 - 2
packages/app/src/views/BrowseView.tsx

@@ -1,7 +1,13 @@
 import React, { useCallback, useState } from 'react'
 import styled from '@emotion/styled'
 import { RouteComponentProps } from '@reach/router'
-import { CategoryPicker, InfiniteVideoGrid, INITIAL_ROWS, Typography, VIDEOS_PER_ROW } from '@/shared/components'
+import {
+  CategoryPicker,
+  InfiniteVideoGrid,
+  INITIAL_ROWS,
+  Typography,
+  INITIAL_VIDEOS_PER_ROW,
+} from '@/shared/components'
 import { colors, sizes } from '@/shared/theme'
 import { useLazyQuery, useQuery } from '@apollo/client'
 import { GET_CATEGORIES, GET_VIDEOS } from '@/api/queries'
@@ -28,7 +34,7 @@ const BrowseView: React.FC<RouteComponentProps> = () => {
     // TODO: don't require this component to know the initial number of items
     // I didn't have an idea on how to achieve that for now
     // it will need to be reworked in some part anyway during switching to relay pagination
-    const variables = { offset: 0, limit: INITIAL_ROWS * VIDEOS_PER_ROW, categoryId: category.id }
+    const variables = { offset: 0, limit: INITIAL_ROWS * INITIAL_VIDEOS_PER_ROW, categoryId: category.id }
 
     if (!selectedCategoryId) {
       // first videos fetch

+ 0 - 33
yarn.lock

@@ -3698,14 +3698,6 @@
   resolved "https://registry.yarnpkg.com/@types/history/-/history-4.7.6.tgz#ed8fc802c45b8e8f54419c2d054e55c9ea344356"
   integrity sha512-GRTZLeLJ8ia00ZH8mxMO8t0aC9M1N9bN461Z2eaRurJo6Fpa+utgCwLzI4jQHcrdzuzp5WPN9jRwpsCQ1VhJ5w==
 
-"@types/hoist-non-react-statics@^3.3.0":
-  version "3.3.1"
-  resolved "https://registry.yarnpkg.com/@types/hoist-non-react-statics/-/hoist-non-react-statics-3.3.1.tgz#1124aafe5118cb591977aeb1ceaaed1070eb039f"
-  integrity sha512-iMIqiko6ooLrTh1joXodJK5X9xeEALT1kM5G3ZLhD3hszxBdIEd5C75U834D9mLcINgD4OyZf5uQXjkuYydWvA==
-  dependencies:
-    "@types/react" "*"
-    hoist-non-react-statics "^3.3.0"
-
 "@types/html-minifier-terser@^5.0.0":
   version "5.1.0"
   resolved "https://registry.yarnpkg.com/@types/html-minifier-terser/-/html-minifier-terser-5.1.0.tgz#551a4589b6ee2cc9c1dff08056128aec29b94880"
@@ -3841,16 +3833,6 @@
   dependencies:
     "@types/react" "*"
 
-"@types/react-redux@^7.1.9":
-  version "7.1.9"
-  resolved "https://registry.yarnpkg.com/@types/react-redux/-/react-redux-7.1.9.tgz#280c13565c9f13ceb727ec21e767abe0e9b4aec3"
-  integrity sha512-mpC0jqxhP4mhmOl3P4ipRsgTgbNofMRXJb08Ms6gekViLj61v1hOZEKWDCyWsdONr6EjEA6ZHXC446wdywDe0w==
-  dependencies:
-    "@types/hoist-non-react-statics" "^3.3.0"
-    "@types/react" "*"
-    hoist-non-react-statics "^3.3.0"
-    redux "^4.0.0"
-
 "@types/react-syntax-highlighter@11.0.4":
   version "11.0.4"
   resolved "https://registry.yarnpkg.com/@types/react-syntax-highlighter/-/react-syntax-highlighter-11.0.4.tgz#d86d17697db62f98046874f62fdb3e53a0bbc4cd"
@@ -3880,13 +3862,6 @@
   dependencies:
     "@types/react" "*"
 
-"@types/redux@^3.6.0":
-  version "3.6.0"
-  resolved "https://registry.yarnpkg.com/@types/redux/-/redux-3.6.0.tgz#f1ebe1e5411518072e4fdfca5c76e16e74c1399a"
-  integrity sha1-8evh5UEVGAcuT9/KXHbhbnTBOZo=
-  dependencies:
-    redux "*"
-
 "@types/source-list-map@*":
   version "0.1.2"
   resolved "https://registry.yarnpkg.com/@types/source-list-map/-/source-list-map-0.1.2.tgz#0078836063ffaf17412349bba364087e0ac02ec9"
@@ -16983,14 +16958,6 @@ redeyed@~2.1.0:
   dependencies:
     esprima "~4.0.0"
 
-redux@*, redux@^4.0.0:
-  version "4.0.5"
-  resolved "https://registry.yarnpkg.com/redux/-/redux-4.0.5.tgz#4db5de5816e17891de8a80c424232d06f051d93f"
-  integrity sha512-VSz1uMAH24DM6MF72vcojpYPtrTUu3ByVWfPL1nPfVRb5mZVTve5GnNCUV53QM/BZ66xfWrm0CTWoM+Xlz8V1w==
-  dependencies:
-    loose-envify "^1.4.0"
-    symbol-observable "^1.2.0"
-
 reflect.ownkeys@^0.2.0:
   version "0.2.0"
   resolved "https://registry.yarnpkg.com/reflect.ownkeys/-/reflect.ownkeys-0.2.0.tgz#749aceec7f3fdf8b63f927a04809e90c5c0b3460"