feat: 218 v5 table component table cell part [05]

[rpt-rfoxy]

Pull request checklist

• Create table cell Component (td)

Detail as per issue below (required):
as perdiscusstion here Discussion
schreenshoot

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
Report missing for 80da58e1	✅ 100.00%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (80da58e)	Report Missing	Report Missing	Report Missing
Head commit (46401a0)	4437	4043	91.12%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#313)	28	28	100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

_{Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more}

Footnotes

1. Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct. ↩

[kurtdoherty]

Thanks @rpt-rfoxy. It feels like you're still stepping beyond the design specification too often. I shouldn't see components be capable of things that design haven't specified. If you have opinions or ideas you'd like to share, by all means raise them on the Figma design spec for the component. And if you do add things into the code that is not specified in the design spec, please highlight it on the PR via a comment.

[kurtdoherty]

question: where does the design specification show the need for flexDirection and isFlexWrap?

[rpt-rfoxy]

hi @kurtdoherty for this question we already discuss here here also hereand its needit since we have limited style properties for the i add new wrapper

inside the td to give flexibility styling.

and if the contentwrapper are necessery we can remove it and also remove flexDirection and isFlexWrap.

[kurtdoherty]

I'm confused. The two comments you have referred to don't seem to have anything to do with this component's need for flexDirection and isFlexWrap...

[rpt-rfoxy]

hi @kurtdoherty i already cahnge the code, and keep the props as DOC from here

[kurtdoherty]

issue: I don't think the special CSS style props defined for ElTableCell should be exposed here. For TableCell, the only way consumers should be able to specify the min/max/fixed width of the cell is via the explicit props for those values, not style.

[rpt-rfoxy]

hi @kurtdoherty thanks for the feedback,
Notes: the minWidth and maxWidth also alignment property is sugested from this discussion

and i just following the past discussion and for flexDirection and isFlexWrap we can remove it since its part of tablecellcontent

[kurtdoherty]

I'm confused here too. My comments above are focused on your use of the TableCellCSSProps type for this React component's style prop. This TableCellCSSProps type includes CSS variable property definitions; these should not be part of TableCell's prop interface, instead they should only be part of ElTableCell's prop interface.

[rpt-rfoxy]

aah i see, im sorry my bad, i'll remove the style tyle,
i thougt since the have child so i thougt it will be good if style props will ocused only for ElTableCellContent.

now its clear, i'll remove it

question: should we remove ElTableCellContent or just keep it?

[kurtdoherty]

issue: please remove the export of this interface and rename the interface as ElTableCellCSSProperties

[rpt-rfoxy]

its laready ermoved

[kurtdoherty]

Thanks @rpt-rfoxy. I still don't understand the need for ElTableCellContent and I don't think you've provided a clear demonstration for why it's necessary, but I'm pretty fatigued from the number rounds all these table-related PRs have taken so I think the best path forward is to just get this merged in and move on.