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

Initial implementation of nimble-table-column-number-text #1414

Merged
merged 44 commits into from
Aug 17, 2023

Conversation

mollykreis
Copy link
Contributor

@mollykreis mollykreis commented Aug 2, 2023

Pull Request

🤨 Rationale

This is part of #1011

This PR adds the initial implementation of the nimble-table-column-number-text with configuration to render the number using a default formatter or as an integer.

There are a number of additional configurations that still need to be added to the column, such as:

  • Text alignment (right vs left)
  • Additional formatting options (decimal and custom)
  • Prefix & suffix
  • Locale-aware formatting

👩‍💻 Implementation

The new column follows the same patterns as other table columns in nimble and uses the existing TableColumnTextCellViewBase and TableColumnTextGroupHeaderViewBase.

More interesting/complex implementation will come in future PRs.

🧪 Testing

  • New unit tests
  • New storybook page (manually tested)
  • New matrix tests

✅ Checklist

  • I have updated the project documentation to reflect my changes or determined no changes are needed.

@mollykreis mollykreis marked this pull request as ready for review August 2, 2023 20:29
@mollykreis
Copy link
Contributor Author

Moving this PR back to draft to address some formatting changes discussed in person:

  • Use 6 digits of precision
  • Remove custom E -> e+ formatting
  • Don't set useGrouping: false

I will also update the HLD to reflect these changes.

@mollykreis mollykreis marked this pull request as draft August 7, 2023 20:43
…table-column-number-text.stories.ts

Co-authored-by: Jonathan Meyer <[email protected]>
@mollykreis mollykreis marked this pull request as ready for review August 16, 2023 16:43
@mollykreis mollykreis enabled auto-merge (squash) August 17, 2023 21:23
@mollykreis mollykreis merged commit 270d7fe into main Aug 17, 2023
4 checks passed
@mollykreis mollykreis deleted the mkreis-number-column branch August 17, 2023 21:38
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.

5 participants