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

fix: remove a div attribute type from TreeTable's prop type #7271

Closed
wants to merge 1 commit into from

Conversation

blan19
Copy link

@blan19 blan19 commented Sep 28, 2024

Defect Fixes

fix #7269 and reflects the div attributes not being applied

Why fix?

In the type declaration, it was stated that div attrs were allow, but these were not actually applied in the TreeTable component.

/**
 * Defines valid properties in TreeTable component. In addition to these, all properties of HTMLDivElement can be used in this component.
 * @group Properties
 */
export interface TreeTableProps extends Omit<React.DetailedHTMLProps<React.InputHTMLAttributes<HTMLDivElement>, HTMLDivElement>, 'onContextMenu' | 'onSelect' | 'ref' | 'value'> { ... }

in the rootProps, div attrs are excluded.
For example, attrs like 'hidden', 'size', etc.

const rootProps = mergeProps(
  {
    role: 'table',
    id: props.id,
    className: classNames(props.className, ptCallbacks.cx('root', { isRowSelectionMode })),
    style: props.style,
    'data-scrollselectors': '.p-treetable-wrapper'
  },
  ObjectUtils.findDiffKeys(props, TreeTable.defaultProps),
  ptCallbacks.ptm('root')
);

i have fix the type declaration for TreeTable's Props to only expose the props allowed in the official documentation, similar to the Props type declaration for Tree.

Copy link

vercel bot commented Sep 28, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
primereact ⬜️ Ignored (Inspect) Visit Preview Sep 28, 2024 4:13pm
primereact-v9 ⬜️ Ignored (Inspect) Visit Preview Sep 28, 2024 4:13pm

@melloware
Copy link
Member

@blan19 check out my PR: #7273

I made it consistent with what Datatable is doing and it had size ignored. It also handled default props correctly.

@blan19
Copy link
Author

blan19 commented Sep 29, 2024

@melloware

maintaining consistency with DataTable seems like a really good. I'll close my PR since it was the wrong direction xD

@blan19 blan19 closed this Sep 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TreeTable: size prop not working, even in official StackBlitz example
2 participants