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] Export GridColumnUnsortedIconProps Interface for custom slots #11472

Conversation

AshishDhama
Copy link

@AshishDhama AshishDhama commented Dec 20, 2023

Fixes #11341

Export GridColumnUnsortedIconProps Interface for custom slots

@mui-bot
Copy link

mui-bot commented Dec 20, 2023

Deploy preview: https://deploy-preview-11472--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against 7b56dc0

@AshishDhama AshishDhama marked this pull request as ready for review December 20, 2023 10:54
@zannager zannager added the component: data grid This is the name of the generic UI component, not the React module! label Dec 20, 2023
@michelengelen michelengelen requested review from MBilalShafi and removed request for cherniavskii December 21, 2023 08:16
@@ -3,7 +3,7 @@ import * as React from 'react';
import { useGridRootProps } from '../../hooks/utils/useGridRootProps';
import { GridSortDirection } from '../../models/gridSortModel';

interface GridColumnUnsortedIconProps extends SvgIconProps {
export interface GridColumnUnsortedIconProps extends SvgIconProps {
Copy link
Member

@MBilalShafi MBilalShafi Dec 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think exporting it this way is not gonna work because:

  1. This component GridColumnUnsortedIcon is a slot and differently exists for material and joy design systems.
  2. Since this file isn't exported like export * from './GridColumnUnsortedIcon', this added export will make no difference anyways.

Possible Options:

  1. We document it somehow and users keep defining on their side. (Like @michelengelen did on csb)
  2. We export it from the specific design entry point, so the users will import it like
    a. import { GridColumnUnsortedIconProps } from '@mui/x-data-grid/material';
    b. import { GridColumnUnsortedIconProps } from '@mui/x-data-grid/joy';
  3. Export them from the root with different names, like GridColumnUnsortedIconPropsMaterial, GridColumnUnsortedIconPropsJoy.

To me 1 and 2 feel better, 3 I would not consider since this will negate the initial idea of isolating the exports for different design systems.
For 2 though, I'm not sure if it'll be discoverable enough for the users, since it will not be obvious from what path they need to import this as we have all other exports from the root path.

Looping @cherniavskii since the problem is a bit broader, should apply to all other design system related exports. Would be nice to hear how you'd approach this. Or if you already thought around this during Joy UI support.

CC @michelengelen

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes absolute sense @MBilalShafi ... maybe it would make sense to add a recipe to the docs instead? We should at least add a short section about this (and similar cases).
WDYT?

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 7, 2024
Copy link

github-actions bot commented Feb 7, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

1 similar comment
Copy link

github-actions bot commented Feb 7, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@michelengelen
Copy link
Member

closing this in favor of #11973

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! PR: out-of-date The pull request has merge conflicts and can't be merged typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DataGrid] Export GridColumnUnsortedIconProps interface for custom slots
5 participants