Conversation
|
✅ Deploy Preview for hpe-theme-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for unrivaled-bublanina-3a9bae ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hpe-design-icons-grommet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR adds proper TypeScript typing to the DataTable Storybook story and improves the data formatting for the percent column. The changes remove the previous workaround (TypedDataTable cast to any) and implement a properly typed solution using generic types from Grommet's DataTable component.
Changes:
- Removed the
TypedDataTableworkaround and added proper generic typing withDatumtype - Converted percent values from strings ("20%") to numbers (20) and added
Intl.NumberFormatfor proper formatting - Reorganized code to define DATA before columns for better type inference
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }, | ||
| ]; | ||
|
|
||
| const percentFormatter = new Intl.NumberFormat('en-US', { |
There was a problem hiding this comment.
This could be helpful for other data stories. Would it be worth adding into a new helpers.tsx under utils?
There was a problem hiding this comment.
yes sure, I can create a follow on issue/PR once the structure of the stories are finalized.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
britt6612
left a comment
britt6612
left a comment
There was a problem hiding this comment.
looks good agree with keeping the
satisfies Meta<typeof DataTable<Datum>>
need to fix merge conflicts
|
Looks good, just need to resolve merge conflicts |
Deploy Preview
What does this PR do?
What are the relevant issues?
Where should the reviewer start?
How should this be manually tested?
Any background context you want to provide?
Screenshots (if appropriate)
Should this PR be mentioned in Design System updates?
Is this change backwards compatible or is it a breaking change?