Skip to content

Frontend Performance Issues #101

Closed
campb303 opened this issue Oct 29, 2020 · 18 comments · Fixed by #207
Closed

Frontend Performance Issues #101

campb303 opened this issue Oct 29, 2020 · 18 comments · Fixed by #207
Assignees
Labels
enhancement Request for a change to existing functionality question Something that requires more information before moving forward

Comments

@campb303
Copy link
Collaborator

There are noticeable stutters on high end machines when opening and closing the sidebar. This is likely due to CSS transitions for width being overly CPU intensive. A more performant solution should be researched.

@campb303 campb303 added enhancement Request for a change to existing functionality frontend question Something that requires more information before moving forward labels Oct 29, 2020
@campb303 campb303 added this to the v1 milestone Oct 29, 2020
@campb303 campb303 self-assigned this Oct 29, 2020
@campb303
Copy link
Collaborator Author

campb303 commented Nov 3, 2020

@dhallett and @seth both recommended that this may be caused by the table needing to do expensive calculations based on the CSS box properties like width per cell and could be reduced by one or both of the following:

  • Removing React, react-table and MUI specific width settings and letting the browser calculate these things
  • Virtualizing the ItemTable to mount content only when it is needed instead of mounting everything at once

@campb303
Copy link
Collaborator Author

This might be addressed by using MUI's Drawer instead of boxes as mentioned in #61

@campb303
Copy link
Collaborator Author

campb303 commented Feb 5, 2021

Usage of the MUI Drawer is still a viable and likely desireable play but virtualizing the ItemTable will likely have the largest effect here. More research of React virtual DOM support with MUI Table is needed and efficient tables is needed.

@campb303 campb303 added the high-priority Needs immediate extra focus label Feb 5, 2021
@campb303 campb303 modified the milestones: v1-proof-of-concept, v2-production-ready-read-only Feb 5, 2021
@campb303 campb303 changed the title Address performance issues with animating the right sidebar opening Frontend Performance Issues Mar 5, 2021
@campb303
Copy link
Collaborator Author

campb303 commented Mar 5, 2021

This item will now be used to address performance related issues.

@campb303
Copy link
Collaborator Author

campb303 commented Mar 10, 2021

Optimizing Performance for React

The DOM

Web pages are not just the HTML we see in the source code. When we write HTML, it is parsed by the browser to create a collection of nodes with mutable properties to be programatically interacted with called the Document Object Model or DOM.

When we load a web page, the following happens:

  • The browser transforms HTML into the DOM
  • CSS is interpreted and DOM node properties are updated
  • JavaScript is loaded and changes that take effect onLoad or when the DOM is finished loading take effect.

The Virtual DOM

Manipulating the DOM is an expensive operation meaning it costs lots of processing power and/or time. To avoid manipulating the DOM, React maintains a separate, in-memory repreentation of the DOM called the Virtual DOM. With the Virtual DOM, changes to the DOM can be efficiently staged and compared with the existing DOM so that only necessary changes to the actual DOM are made keeping things fast.

Wasted Renders and Expensive Operations

React components are just JavaScript functions that render HTML which in turn is transformed into nodes within the DOM. This process is referred to as rendering.

By default, React will re-render every component every time anything changes. This is appropriate when the value of props change but undesirable when the value of props stays the same. When a component is re-rendered but the value of its props stays the same its called a wasted render. Wasted renders are to be avoided in most cases because a wasted render results in unneccesary work to modify the DOM.

Regular functions can also be expensive. We equally want to avoid re-running expensive functions if the inputs don't change.

In general, its a bad idea to re-run a function with the same inputs to produce an output we already have. It would be faster just to use the output we already have.

Memoization

Memoization is an optimization technique for improving performance with expensive functions. If the inputs to a function are the same as the last time the function was run, then the last result is returned instead of re-running the function. In React, there are three different typoes of memoization:

Memoizing a Component

Both React.PureComponent and React.memo can be used to memoize a component and help with preventing wasted renders. Because our project primarily uses function components, I will focus on React.memo.

React.memo is a high order component meaning it takes a compenent as one of its arugments and returns a component. To use React.memo we simply wrap our function components in React.memo:

// Declare Component to be Memoize
const PrintArrayComp = ({ array }) => array.map( 
    (item, index) => <p>{`[${index}] ${item}`}</p>
);

// Memoize Component
const PrintArray = React.memo(PrintArrayComp);

// Use Component
<PrintArray array={["apple", "orange", "banana"]} />

By default, React.memo will compare the old props to the new props using referential equality as opposed to value equality. We can override this behavior if we'd like by passing a second argument to React.memo containing a function that accepts the old and new props and returns a boolean value to say whether or not the props should be considered equal. Using our PrintArray component above, we can override the default behavior to consider props equal if the array is the same length:

// Declare Component to be Memoize
const PrintArrayComp = ({ array }) => array.map( 
    (item, index) => <p>{`[${index}] ${item}`}</p>
);

// Declare Prop Comparison Override Function
const propsAreEqual = (prevProps, nextProps) => prevProps.length === nextProps.length

// Memoize Component
const PrintArray = React.memo(PrintArrayComp, propsAreEqual);

// Use Component
<PrintArray array={["apple", "orange", "banana"]} />

Read Pure Functional Components in React for more details.

Memoizing Props

In most cases, the default referential equality comparsion of React.memo is what we want but we need to ensure referential equality for this to work. To do so we have to separate our props into two groups:

  • Primitive Data: Boolean. null, undefined, String and Number.
  • Non-primitive Data: Arrays and Objects
  • Functions Any function

Primitive data does not need to be memoized.

Memoizing Non-Primitive Data

We can use React.useMemo to memoize non-primitive data bypassing a function that generates the data (a factory function) as the first argument and an array of values that should be watched for change before re-running our factory function (a dependency list) as the second argument:

// Declare memoized component to print an object as a table
const ObjectToTable = React.memo(({object}) => (
    object.keys.map( (key) => {
        let value = object.key;
        return <b>{key}</b>: {value};
    })
));

// Memoize data to pass to component
const objectFromAPI = React.useMemo(
    (apiParams) => getObject(apiParams),
    [apiParams]
);

// Use memoized data with component
<ObjectToTable object={objectFromAPI} />

Memoizing Functions

We can use React.useCallback to memoize a function. The syntax is imilar to React.useMemo where we pass an argument as the first argument and a dependency list as the second argument but it differs in that React.useMemo returns the result of a function and React.useCallback returns the function itself to be run later.

// Declare component that accepts an onClick event handler
const AlertOnClick = React.memo( ({clickHandler}) => (
    <p onClick={clickHandler}>Click me!</p>
));

// Declare alert message
// This does not need memoized because it is primitive data
const alertMessage = "I've been clicked!";

// Memoize the callback
const clickHandler = React.useCallback( _ => alert(alertMessage), [alertMessage]);

// Use memoized callback with component
<AlertOnClick clickHandler={clickHandler}>

For a more in depth look at React.memo, React.useMemo and React.useEffect see Introduction to React.memo, useMemo and useCallback.

Optimization Always Comes At A Cost

Optimization is not magic, it is a tradeoff and should be used only when appropriate. For a detailed look at when and when not to use React.useMemo and React.useCallback, read When to useMemo and useCallback.

Further Reading

@campb303 campb303 removed the frontend label Mar 17, 2021
@campb303 campb303 removed the high-priority Needs immediate extra focus label Mar 29, 2021
@campb303
Copy link
Collaborator Author

Performance issues will likely need to be addressed by refactoring existing components with composable designs by utilizing the React children prop.

@wrigh393
Copy link
Collaborator

wrigh393 commented Apr 2, 2021

One of the proposed solutions to improving performance is to implement virtualization in the ItemTable component. Based on some research the two most commonly used libraries for this are react-virtualized and react-window. These libraries were created by the same author but have a few differences. I'll explain what these libraries do and how the two differ.

What do they do?

Both of these libraries are used to implement windowing into a project. Windowing or virtualization improves the performance of a long list (in our case this is the rows of the table) by only writing what the user can see into the DOM. Currently, the ItemTable component doesn't use windowing so in order to show even one row from the table React would have to write every single row to the DOM before we can even see the one row. For example, if we have 1,000 items in the table before a user could see even one item from the table all 1,000 items before we can see the first item.

How do they differ?

According to the author

react-window is a complete rewrite of react-virtualized that is focused on being smaller and faster.

Because of this react-window has less features than react-virtualized. Below you will find a list of all the components that are available in each library and a link to the documentation for that component:

react-window

react-virtualized

The author recommends using react-window instead of react-virtualized if it meets the needs of the project but if not use react-virtualized. You can also create decorators for the react-window components that add the functionality that you need. Some examples of how this works can be found in the examples shown in this article.

Conclusion

From the research that I've done my understanding is that react-window should give us the functionality that we are looking for. If not the option is there for us to use react-virtualiztion or create decorators to add the specific functionality that's missing from react-window. For the next steps, I think that we should discuss the two options to see which is the most viable option.

@wrigh393
Copy link
Collaborator

wrigh393 commented Apr 9, 2021

Starting work on virtualizing the ItemTable

@wrigh393
Copy link
Collaborator

wrigh393 commented Apr 9, 2021

A rough version of virtualization has been implemented in the ItemTable component. This comment will detail how it was implemented.

Virtualization Library

I decided to use react-window for the library as it is the smaller of the two and seemed to provide all the functionality that we would need.

Implementing Virtualization

To implement virtualization the the children of the TableBody component needed to be changed. In the current ItemTable, the rows are immediately mapped over and render a row for each item and a cell for each column of the table:

  <TableBody {...getTableBodyProps()}>
// Map over the array of rows (items) 
                        {rows.map((row) => {
                            prepareRow(row);
                            let isSelected = selectedRow.queue === row.original.queue && selectedRow.number === row.original.number
                            return (
                                <TableRow
                                    hover
                                    onClick={() => {
                                        history.push(`/${row.original.queue}/${row.original.number}`);
                                        setSelectedRow({ queue: row.original.queue, number: row.original.number });
                                    }}
                                    // This functionality should be achieved by using the selected prop and
                                    // overriding the selected class but this applied the secondary color at 0.08% opacity.
                                    // Overridding the root class is a workaround.
                                    classes={{
                                        root: (isSelected && rowCanBeSelected) ? classes.rowSelected : classes.bandedRows,
                                        hover: classes.hoverBackgroundColor
                                    }}
                                    {...row.getRowProps()}
                                >
//Map over the array of cells for each item
                                    {row.cells.map(cell => (
                                        cell.render(_ => {
                                            switch (cell.column.id) {
                                                case "dateReceived":
                                                    return (
                                                        <ItemTableCell TableCellProps={cell.getCellProps()}>
                                                            <RelativeTime value={cell.value} />
                                                        </ItemTableCell>
                                                    );
                                                case "lastUpdated":
                                                    return (
                                                        <LastUpdatedCell
                                                            time={cell.value}
                                                            ItemTableCellProps={cell.getCellProps()}
                                                        />
                                                    );
                                                default:
                                                    return (
                                                        <ItemTableCell TableCellProps={cell.getCellProps()}>
                                                            {cell.value}
                                                        </ItemTableCell>
                                                    );
                                            }
                                        })
                                    ))}
                                </TableRow>
                            );
                        })}
                    </TableBody>

The only thing that needed to be added for virtualization was wrapping the rows in the FixedSizeList component or the VariableSizeList component from react-window. In this case, I used the FixedSizeList component. The difference between the two is that the VariableSizeList can estimate the size of a row. This may be useful in the future but for the sake of testing the implementation of virtualization, I went with the less complex component.

The FixedSizeList component requires these props to be passed:

  • children: component - the component that renders the actual item. In our case, this is where the TableRow component will go.
  • height: number | string - The height of the list. Since we are using a vertical list this MUST be a number. If we were using a horizontal list we could pass a string value (ex. "50%")
  • itemCount: number - Total number of items in the list. In our case, we are passing the length of the array of items.
  • itemSize: number - Size of an item in the direction being windowed. For vertical lists, this is the row height. For horizontal lists, this is the column width.
  • width: number | string - The width of the list. For horizontal lists, this must be a number. It affects the number of columns that will be rendered (and displayed) at any given time. For vertical lists, this can be a number or a string (e.g. "50%").

It also provides two methods for us to use:

  • scrollTo(scrollOffset: number): void - Scroll to the specified offset (scrollTop or scrollLeft, depending on the direction prop).

  • scrollToItem(index: number, align: string = "auto"): void - Scroll to the specified item. By default, the List will scroll as little as possible to ensure the item is visible. You can control the alignment of the item though by specifying a second alignment parameter. Acceptable values are:

    • auto (default) - Scroll as little as possible to ensure the item is visible. (If the item is already visible, it won't scroll at all.)
    • smart - If the item is already visible, don't scroll at all. If it is less than one viewport away, scroll as little as possible so that it becomes visible. If it is more than one viewport away, scroll so that it is centered within the list.
    • center - Center aligns the item within the list.
    • end - Align the item to the end of the list (the bottom for vertical lists or the right for horizontal lists).
    • start - Align the item to the beginning of the list (the top for vertical lists or the left for horizontal lists).

I didn't use either of the provided methods but these may be useful in the future as well.

I also implemented memorized list item as shown in the docs for react-window. In order to implement memorization you need to pass the provided comparison function, areEqual as the second argument of the memoized row. The areEqual function knows to compare individual style props and ignore the wrapper object in order to avoid unnecessarily re-rendering when cached style objects are reset. In the future, I will need to look into how beneficial memoizing the rows is to performance improvements. Here the code for how virtualized and memoized row were implemented in the ItemTable:

<TableBody {...getTableBodyProps()}>
+                            <FixedSizeList
+                                height={800}
+                                itemCount={rows.length}
+                                itemSize={120}
+                                width="100%"
+                            >
+                                {memo(({ index, style }) => {
+                                   const row = rows[index]
                                    prepareRow(row)
                                    let isSelected = selectedRow.queue === row.original.queue && selectedRow.number === row.original.number
                                    return (
                                        <TableRow
                                            style={style}
                                            hover
                                            onClick={() => {
                                                history.push(`/${row.original.queue}/${row.original.number}`);
                                                setSelectedRow({ queue: row.original.queue, number: row.original.number });
                                            }}
                                            // This functionality should be achieved by using the selected prop and
                                            // overriding the selected class but this applied the secondary color at 0.08% opacity.
                                            // Overridding the root class is a workaround.
                                            classes={{
                                                root: (isSelected && rowCanBeSelected) ? classes.rowSelected : classes.bandedRows,
                                                hover: classes.hoverBackgroundColor
                                            }}
                                            {...row.getRowProps({ style, })}
                                        >
                                            {row.cells.map(cell => (
                                                cell.render(_ => {
                                                    switch (cell.column.id) {
                                                        case "dateReceived":
                                                            return (
                                                                <ItemTableCell TableCellProps={cell.getCellProps()}>
                                                                    <RelativeTime value={cell.value} />
                                                                </ItemTableCell>
                                                            );
                                                        case "lastUpdated":
                                                            return (
                                                                <LastUpdatedCell
                                                                    time={cell.value}
                                                                    ItemTableCellProps={cell.getCellProps()}
                                                                />
                                                            );
                                                        default:
                                                            return (
                                                                <ItemTableCell TableCellProps={cell.getCellProps()}>
                                                                    {cell.value}
                                                                </ItemTableCell>
                                                            );
                                                    }
                                                })
                                            ))}
                                        </TableRow>
                                    );
+                                }, areEqual,
+                                    [prepareRow, rows]
+                               )}
+                            </FixedSizeList>
  </TableBody>

So far this has provided a good performance boost but it will still need some tweaks as this change will impact the UI.

@wrigh393
Copy link
Collaborator

Currently, the ItemTable is virtualized but the table is rendering multiple scrollbars. I believe this is due to the virtualized table haveing its own scrollbar and then the presence of the scrollbar for the entire window. I fixed this issue on the X-axis of the table by setting the width property of the FixedSizeList to be equal to window.innerWidth. Here is the definition of what this prop does from the Mozilla MDN Web Docs

The read-only Window property innerWidth returns the interior width of the window in pixels. This includes the width of the vertical scrollbar if one is present.
More precisely, innerWidth returns the width of the window's visual viewport. The interior height of the window—the height of the layout viewport—can be obtained from the innerHeight property.

Because this takes the scrollbar into account when calculating the width I was able to stop the scrollbar from rendering when it wasn't necessary. I tried to use this same principle to stop the extra vertical scrollbar from rendering. but this causes the entire page's vertical scroll to be controlled with the same scrollbar instead of just the table being scrollable.

The react-window docs mention that it can use the react-virtualized-auto-sizer package to have a list fill 100% of the width or height of a page, but I have been unable to get the list to render the data when using this package. I have looked at other alternatives that will allow the list to take up the entirety of its container without rendering extra scrollbars such as this example. None of the solutions I have found so far work and finding the reason for why they don't work will take further research.

@wrigh393
Copy link
Collaborator

Working on this.

@wrigh393
Copy link
Collaborator

After finding a reply to this issue here I was able to make progress on finding a solution to the multiple scrollbar problem. To summarize the comment and the docs it references:

The AutoSizer component grows to fill the available space but it won't make that space larger. So if the component does not grow or is too large then the styling of the parent element needs to be changed. This can be achieved by providing a class to the TableBody component that gives either height:100% or flex: 1 as suggested in the docs.

We should be able to render the table using these while keeping all of the components present. The next step is to find the proper implementation of these suggested style changes and see if they cause the table to render in the expected fashion.

@wrigh393
Copy link
Collaborator

Here is a tutorial on how I implemented the AutoSizer component from react-window

Implementing the react-window

Implementing the AutoSizer component requires you to wrap the FixedListComponent with the component.

<AutoSizer>
 //Function responsible for rendering children. 
//This function should implement the following signature: ({ height: number, width: number }) => PropTypes.element

 {({height, width}) => (
      <List
        height={height}
        rowCount={list.length}
        rowHeight={20}
        rowRenderer={rowRenderer}
        width={width}
      />
    )}
  </AutoSizer>,

The height and width props are determined based on the value of the parent. The child of the component need to be passed as a component.

The problem that we are facing is that the TableBody that wraps the both the AutoSizer and FixedSizeList has a height of 0. Because of this we need to find a way to pass a height. Typically this could be done by passing a value of 100% but because the parent has a height of 0 this will not work. We could give the component a class that sets the height of the AutoSizer but that may not be a solution that works on all screen sizes.

There is also a problem we are facing related to web accessibility standards. The component that react-window uses to render is a div and not a table element. This is something that we would have to address in order to meet required accessibility standards.

@wrigh393
Copy link
Collaborator

After some help from @seth we were able to apply the styles we wanted to the actual TableBody component. This was accomplished by setting the display of the TableBody component to table-row. What this does is allow the TableBody component to behave like a <tr> element. This is helpful because by default most browser give the <tbody> component a height of 0. With the display set to table-row the values that we pass for the height are taken into account when calculating the height.

This is also is caused by how AutoSizer works. Here is how it functions as described in the docs:

AutoSizer expands to fill its parent but it will not stretch the parent.
This is done to prevent problems with flexbox layouts.
If AutoSizer is reporting a height (or width) of 0 then it's likely that the parent element (or one of its parents) has a height of 0.
(eg You may need to add height: 100% or flex: 1 to the parent.)

So because the TableBody had no height setting a height allowed AutoSizer to work as intended.

@wrigh393
Copy link
Collaborator

Working on this.

@wrigh393
Copy link
Collaborator

As mentioned in this comment from #133. The ItemTable re-render every time an item is selected. This happens because the Hooks for the ItemTable change as well as the rowCanBeSelected prop changing. Based on this section of the react-table docs I believe that this is where the Hooks change and based on this comment from the author it seems intended:

No you're not missing anything. This response honestly deserves a blog post, but I'll have to link to another one for now: https://kentcdodds.com/blog/react-hooks-pitfalls. Specifically read the section on "Pitfall 4: Overthinking performance".
In a nutshell, rerenders are not bad at all. They are what keep your UI up to date. Expensive rerenders, however, can become a problem very quickly. Luckily, React Table v7 is built on this exact premise that re-renders are good, but expensive rerenders are bad.
React Table is rerender heavy. This ensures things stay up to date.
React Table doesn't use PureComponent or rerender bailouts. If you want to use these in your own markup, feel free to do so, but at your own risk. In most, if not all, scenarios you would do this, it could probably be solved by using a useMemo or useCallback
React Table uses extensive memoization, caching, and change-detection to ensure that re-renders are extremely performant.
Moral of the story: Only worry about rerenders if they are expensive or degrading performance.

While this is an issue now the author of react-table spoke about a new version that will not have these issues in this comment.

The rowCanBeSelected prop changes whenever the sidebar is opened or closed and is used to add the selected styles to the row that is selected. The best way to solve this issue would be to find a better way to determine if a row is selected or not.

@campb303 campb303 linked a pull request Apr 22, 2021 that will close this issue
@wrigh393
Copy link
Collaborator

wrigh393 commented Apr 28, 2021

This comment will be a more clear explanation of virtualization and its impact on performance.

What slows down a React app?

There are two many places where performance is impacted within a React app, the browser and React itself. Here are some of the specific issues that can cause performance issues for each area.

Browser

  • DOM elements and mutations
    • Creating a lot of DOM elements or doing a lot of DOM mutations frequently can cause performance issues
  • Repaints and Reflows
    • This refers to the browser needing to rerender (or paint) an element on the screen as it changes but does not impact the layout, for example, changing the outline of an element or changing the background color would cause a repaint. This is expensive because the browser needs to then check the visibility of all other DOM nodes.
    • Reflow is the process in which the browser recalculates the positions and geometry of elements so that they can be rerendered. Reflow time is changed by various properties like DOM depth, CSS rule efficiency, and style changes.

React

  • Unnecessary renders
    • These are renders that result in the same DOM. This can be avoided by memoizing components so that components shallow compare state and props helping them to only change when necessary.

What is virtualization/windowing?

Virtualization means that the app only creates DOM elements that the user can see. A good example of how this works can be seen with Kindles or the Kindle app. When reading books through kindle the entire book's information is there but instead of using resources to render the whole book it instead renders just one page.

How does react-window approach virtualization?

react-window and other windowing libraries implement virtualization by placing a large scrollable div inside of a smaller div and then absolutely positioning the visible items based on the user's scroll position. As the user continues to scroll the library will calculate what items are in view a position them accordingly.

Rendering a virtualized list

Rendering a virtualized list with react-window is fairly simple. Essentially all that needs to happen is that you pass a function for rendering rows as the child of one of the components in the library for rendering the list. In our case this is the FixedSizeList component. FixedSizeList requires these props:
The FixedSizeList component requires these props to be passed:

  • children: component - the component that renders the actual item. In our case, this is where the TableRow component will go.
  • height: number | string - The height of the list. Since we are using a vertical list this MUST be a number. If we were using a horizontal list we could pass a string value (ex. "50%")
  • itemCount: number - Total number of items in the list. In our case, we are passing the length of the array of items.
  • itemSize: number - Size of an item in the direction being windowed. For vertical lists, this is the row height. For horizontal lists, this is the column width.
  • width: number | string - The width of the list. For horizontal lists, this must be a number. It affects the number of columns that will be rendered (and displayed) at any given time. For vertical lists, this can be a number or a string (e.g. "50%").

It is important to note that you need to pass the style and index props to the function that renders the items of the list. The style prop is important because react-window absolutely positions elements via inline styles so this prevents any problems with things like rows being blank while scrolling. The index prop is important because it lets react-window know where that item is in relation to other items in the list.

Here is a code example of how this may look:

const array =[...]  //this is the data

function rowRenderer ({ index, style }) {
return(
<div style={style}>
  {array [index]}
</div>
}
function renderList () {
  return(
   <FixedSizeList
       height={800}
       itemCount={rows.length}
       itemSize={120}                               
       width="100%"
>
{rowRenderer}
 </FixedSizeList>
  )
}

You can also implement the AutoSizer component from react-virtualized so that the height and width of the list can be calculated as the browser size changes.

Implementing AutoSizer is pretty simple. Here is an example:

function rowRenderer ({ index, style }) {
return(
<div style={style}>
  {array [index]}
</div>
}
function tableRenderer() {
  return (
    <AutoSizer>
      //Function responsible for rendering children. 
     //This function should implement the following signature: ({ height: number, width: number }) => PropTypes.element
         {({ height, width }) => (
              <FixedSizeList
                   height={height}
                   rowCount={list.length}
                   rowHeight={20}
                   width={width}
              >
                   {rowRenderer}
              </FixedSizeList>
    )}
  </AutoSizer>,
  )
}

It's important to note that AutoSizer calculates its height and width from its parent element so if the parent element doesn't have a height or width AutoSizer will not have those either so be sure that its parent element has those props.

Conclusion

Virtualization is a great way to speed up performance for apps that need to load long lists of data. react-window is pretty easy to get set up and provides plenty of helpful features. If react-window doesn't meet all your needs you can also use react-virtualized which was created by the same author but is a lot larger in size. react-virtualized provides components for a larger set of use cases but both libraries share enough similarities that they should be useable interchangeably for most projects..

@campb303 campb303 removed their assignment Apr 28, 2021
@campb303
Copy link
Collaborator Author

campb303 commented May 4, 2021

The current PR for this is workable but un-maintainable.

  • AutoSizer does not behave the way that we expect it to and this is made up for with hard coded CSS values for the table.
  • The combination of FixedSizeList and AutoSizer create invalid DOM nesting with 2-4 extra div elements that break styling an accessibility standards.
  • Style elements are being unnecessarily duplicated in multiple places making the code sloppy and WET
  • The table component itself has become way too large for management by us, those who wrote it, and will be a nightmare for anyone else. It needs to be refactored.

So far, some helpful resources that I have come across are:

  • Rendering Large Lists w/ React Window
    This article details what exactly react-window does with helpful graphics and a breakdown of the neccesary refactoring to make it work. It does not, however, go into how to use AutoSizer.
  • Example of react-table with virtualization using react-window
    This Codesandbox demonstrates how to use a fixed size table from react-table via react-window which could be useful if we decide to stay with react-table. However, the code example is still difficult to follow and relies on div's with ARIA roles as opposed to semantic HTML tags.
  • react-window FixedSizeList Docs
    The docs for FixedSizeList show all of the available props including outerElementType and innerElementType which can be used to render the above mentioned div's with other valid DOM elements like tr or tbody. However, these props map to explicit document.createElement calls and do not accept JSX components.
  • MUI Virtualized Table on GitHub
    This project, if it works as well as it looks like it does, might solve our issues. It used MUI's Table components and react-virtualized to create a virtualized MUI table with very simple syntax and a plethora of override options. It also supports passing JSX components as overrides, percentage measurements for height and width of the table and more. The project looks to be minimally maintained and the long term viability needs to be assessed but this might be the best way forward for a clean implementation.

For now, our code is ugly but functional. This is now the core of our issues with the frontend and needs to be addressed. We are not release candidate ready let alone v1.

@campb303 campb303 closed this as completed May 5, 2021
Sign in to join this conversation on GitHub.
Labels
enhancement Request for a change to existing functionality question Something that requires more information before moving forward
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants