Proposal: prop to remove detail panel icon

[peacechen]

The detail panel icon takes up about 1/2 to 2/3 of a regular column width. For situations with a large number of columns, I'd like to add an option to remove that column entirely.

I can set the icon to an empty component, but the column still exists and takes up space.

Additionally, I propose adding a prop to automatically expand the details panel on row click. This can currently be done using the togglePanel() param in the onRowClick callback. It would be convenient for a prop to take care of that automatically.

@oze4 You had mentioned that there shouldn't be any new features added during the refactor. Is this a feature that I can add now, or will it have to wait?

[oze4]

I'm not sure I follow. You want to use the detail panel but have no column for the button (which allows you to open said detail panel)? How are you going to open it?

[oze4]

Ultimately, I would say to keep this in the back of your mind.

I wouldn't say this is a feature exactly, it's more of an abstraction to make it easier on the consumer. Due to the fact this doesn't resolve a critical bug, or add a feature to solve a critical issue, this will be put on the back burner for a little while.

Additionally, I propose adding a prop to automatically expand the details panel on row click. This can currently be done using the togglePanel() param in the onRowClick callback. It would be convenient for a prop to take care of that automatically.

Personally, I like the idea of "no magic" (or keeping things as un-opinionated as possible) internally, and allowing the user to accomplish things like this as you are currently accomplishing them.

[peacechen]

Was a release made with the changes you mentioned above? I can hide the icon by passing in a blank element, but the column still renders. The goal is to eliminate that column to reclaim space for other data columns.

[oze4]

I thought I did but...yeah. I just published v2.3.23, which should have the updated code above which at least makes it easier to not show the icon.

It's not an issue with misunderstanding the goal, rather, manpower. I would rather focus my time working out the kinks and updating the codebase vs adding props/features like this that will be rewritten in a matter of weeks anyway.

Not that these issues are a waste of time, just that issues like this are not being prioritized (due to the fact this is more "sugar" than "resolving a critical bug"). There are hacks (whether css or what have you) that you could put in place on your end for the next couple of weeks. We apologize for the inconvenience.

See here for our roadmap.

In summary, all updates to v2 have stopped, outside of critical bugs.

As far as the feature request, it will have to be addressed at a later time (unless someone else wants to add it - for now I am unable to focus on things like this). Thank you so much for understanding!

[peacechen]

I totally understand the shortage of manpower. I'm happy to submit a PR with this change, but I need to know which branch to work off of. Every project has technical debt and there's usually a desire to rewrite/refactor. I worry that if this phase takes more than a few weeks, this project may lose favor as people peel off to other forks that are adding features.

[oze4]

The main branch. I'm not in this to generate interest, I'm in it to help make this better. If someone wants to use a diff library because we don't have X features until couple weeks, they have every right to do so. I won't lose sleep over it.

[peacechen]

It's unlikely that mbrn will return to manage the original repo anytime soon. Even if he does, it's not sustainable for a widely used project to be hamstrung by one person.
I view this project's goal as helping active users who depend on it in their products. Part of that is long term refactors to sustain it, and another part is ongoing features and bugfixes. We need to build a community of contributors, and that will only happen when there are more people using this.

[peacechen]

With the delete icon (part of the editing features of the table), the detail panel icon real estate adds up. That's a good 1.5-2 columns worth of space.

When I get a chance to work on this, I'll make all the icons optional. If they're passed in as null, it won't render itself or the container & column it's in.

[oze4]

Yea I agree. How do you think we should fix it (what needs to be changed in the src code)? I'd be happy to merge any PR(s) for this!

[peacechen]

I went to implement this and noticed that you added the conditional to m-table-body-row.js:
https://github.com/material-table-core/core/blob/master/src/components/m-table-body-row.js#L161

Hopeful that your change would do the job, I first tried to pass in a falsy value for the DetailsPanel icon. Unfortunately, TS complains because the icons need to be wrapped in a forwardRef like so:

export const MaterialTableIconsWithoutDetail: Icons = {
  ...
  DetailPanel: forwardRef((props, ref) => null),
};

That prevents the icon from rendering, but the container button continues to render and take up space. We can't check for falsy props.icons.DetailPanel because it must be a function, even if that function returns null.

The only practical way to achieve this is to add a prop such as options.showDetailPanelIcon:

    if (!this.props.options.showDetailPanelIcon) {
      return null;
    }

I'll create a PR for this. showDetailPanelIcon will default to true.

[oze4]

Thank you! @Domino987 just wanted to get this on your radar. Cheers.

[peacechen]

Submitted PR #136

[Domino987]

Look good to me. Thank you.

[peacechen]

@oze4 @Domino987
I submitted an additional PR #144 to fix the header row's alignment when showDetailPanelIcon is disabled. There are a few other PRs merged or in the pipeline that would valuable in the next release. When do you expect the next release will be?

[oze4]

#144 has been merged