Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DataGrid] hideFooter does not hide custom footer #821

Closed
2 tasks done
GlennSmeulders opened this issue Jan 8, 2021 · 6 comments
Closed
2 tasks done

[DataGrid] hideFooter does not hide custom footer #821

GlennSmeulders opened this issue Jan 8, 2021 · 6 comments
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@GlennSmeulders
Copy link

GlennSmeulders commented Jan 8, 2021

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

When overriding the footer with the 'components' property, I can no longer hide it with 'hideFooter' property.

Expected Behavior 🤔

I would expect that 'hideFooter ' would hide the footer, regardless if it's the standard footer or a custom one.

Steps to Reproduce 🕹

I've created a snippet here to demonstrate: https://codesandbox.io/s/material-demo-forked-e4cjg?file=/demo.tsx

Steps:

  1. Override standard footer by using the 'components' property and supply a value for 'footer'
  2. Enable hideFooter on the DataGrid component
  3. Footer is still visible

Context 🔦

Our footer contains actions that should only be available to certain users. For read-only users I want to hide the footer completely because the footer contains actions that read-only users should not be aware of.

@GlennSmeulders GlennSmeulders added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jan 8, 2021
@DanailH
Copy link
Member

DanailH commented Jan 8, 2021

Hi, @GlennSmeulders thanks for reporting this. Currently, the hideFooter prop only works with the default footer as if you provide your own implementation of the footer the default one is completely overwritten (just a side note, the Header component behaves the same). Can you explain your motivation of why would expect a prop to overwrite the custom implementation that you provide? Our logic was that if you were to provide your own implementation it is also likely that you would need some specific interaction and the props may not be enough.

@GlennSmeulders
Copy link
Author

Hi @DanailH, thanks for your comment. Going from the documentation on 'hideFooter' it states 'if true, the footer component is hidden'. If you want to override the footer you have to do it trough components -> footer. So in my mind I thought 'hideFooter' would just hide the footer component in general, regardless if it's overridden or not.

To me, overriding the footer means overriding the content that is placed in the footer. 'hideFooter' property can stay functional and there is no misunderstanding of what it does.

We could also update the documentation so that 'hideFooter' states: 'if true, the default footer component is hidden'.

Let me know what you think.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 8, 2021

So in my mind I thought 'hideFooter' would just hide the footer component in general, regardless if it's overridden or not.

I had the same assumption, it seems more intuitive. The mental model I had built is that Footer is a slot (customizable) that hideFooter can disable.

@DanailH
Copy link
Member

DanailH commented Jan 8, 2021

Alright, yes both points make sense. I agree that it is logical for the hideFooter to work on the custom component as well (the same will apply to the Header component).

@DanailH DanailH added bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jan 8, 2021
@oliviertassinari
Copy link
Member

@GlennSmeulders What do you think about this diff?

diff --git a/packages/grid/_modules_/grid/GridComponent.tsx b/packages/grid/_modules_/grid/GridComponent.tsx
index 3608c42..0b17faa 100644
--- a/packages/grid/_modules_/grid/GridComponent.tsx
+++ b/packages/grid/_modules_/grid/GridComponent.tsx
@@ -186,18 +186,20 @@ export const GridComponent = React.forwardRef<HTMLDivElement, GridComponentProps
                     </GridDataContainer>
                   </GridWindow>
                 </div>
-                <div ref={footerRef}>
-                  {customComponents.footerComponent || (
-                    <DefaultFooter
-                      paginationComponent={
-                        !!gridState.options.pagination &&
-                        gridState.pagination.pageSize != null &&
-                        !gridState.options.hideFooterPagination &&
-                        (customComponents.paginationComponent || <Pagination />)
-                      }
-                    />
-                  )}
-                </div>
+                {gridState.options.hideFooter ? null : (
+                  <div ref={footerRef}>
+                    {customComponents.footerComponent || (
+                      <DefaultFooter
+                        paginationComponent={
+                          !!gridState.options.pagination &&
+                          gridState.pagination.pageSize != null &&
+                          !gridState.options.hideFooterPagination &&
+                          (customComponents.paginationComponent || <Pagination />)
+                        }
+                      />
+                    )}
+                  </div>
+                )}
               </ApiContext.Provider>
             </ErrorBoundary>
           </GridRoot>
diff --git a/packages/grid/_modules_/grid/components/DefaultFooter.tsx b/packages/grid/_modules_/grid/components/DefaultFooter.tsx
index 682cb80..c357688 100644
--- a/packages/grid/_modules_/grid/components/DefaultFooter.tsx
+++ b/packages/grid/_modules_/grid/components/DefaultFooter.tsx
@@ -20,10 +20,6 @@ export const DefaultFooter = React.forwardRef<HTMLDivElement, DefaultFooterProps
     const options = useGridSelector(apiRef, optionsSelector);
     const selectedRowCount = useGridSelector(apiRef, selectedRowsCountSelector);

-    if (options.hideFooter) {
-      return null;
-    }
-
     const showSelectedRowCount =
       !options.hideFooterSelectedRowCount && selectedRowCount > 0 ? (
         <SelectedRowCount selectedRowCount={selectedRowCount} />

Do you want to work on a pull request? :)

@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Jan 9, 2021
dtassone added a commit to dtassone/material-ui-x that referenced this issue Jan 12, 2021
@dtassone
Copy link
Member

fixed in PR #846

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

No branches or pull requests

4 participants