feat(compass-collection): Document Count Screen followups – CLOUDP-348924 #7408
Conversation
- Create shared compactBytes() and compactNumber() utilities - Remove numeral dependency from compass-collection, compass-crud, and compass-indexes - Use centralized formatting for consistent SI notation across packages - Update tests to match new formatting (e.g., '100.0 kB' instead of '100.0KB') - Fix webpack-config-compass browserslist for Electron 37.6 compatibility
There was a problem hiding this comment.
Pull Request Overview
Centralizes formatting utilities and removes numeral dependency from multiple packages as part of Document Count Screen followups.
- Centralizes byte and number formatting by creating shared utilities in compass-components
- Replaces numeral usage across compass-collection, compass-crud, and compass-indexes packages
- Removes defensive schema analysis check from document count screen
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/compass-components/src/utils/format.ts | Adds centralized compactBytes and compactNumber formatting utilities |
| packages/compass-components/src/index.ts | Exports new formatting utilities |
| packages/compass-indexes/src/plugin-title.tsx | Replaces numeral with centralized formatting helpers |
| packages/compass-indexes/src/components/regular-indexes-table/size-field.tsx | Updates size formatting to use compactBytes utility |
| packages/compass-crud/src/plugin-title.tsx | Replaces numeral with centralized formatting helpers |
| packages/compass-collection/src/components/mock-data-generator-modal/document-count-screen.tsx | Updates byte formatting and removes defensive schema check |
| Multiple package.json files | Removes numeral dependency from 3 packages |
| Test files | Updates test expectations to match new formatting output |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| const units = si | ||
| ? ['B', 'kB', 'MB', 'GB', 'TB', 'PB', 'EB', 'ZB', 'YB'] | ||
| : ['B', 'KiB', 'MiB', 'GiB', 'TiB', 'PiB', 'EiB', 'ZiB', 'YiB']; | ||
| const i = Math.floor(Math.log(bytes) / Math.log(threshold)); | ||
| const num = bytes / Math.pow(threshold, i); | ||
| return `${num.toFixed(decimals)} ${units[i]}`; |
There was a problem hiding this comment.
Math.log(bytes) will throw an error or return -Infinity for bytes <= 0. The function should handle negative values and zero case consistently.
| const units = si | |
| ? ['B', 'kB', 'MB', 'GB', 'TB', 'PB', 'EB', 'ZB', 'YB'] | |
| : ['B', 'KiB', 'MiB', 'GiB', 'TiB', 'PiB', 'EiB', 'ZiB', 'YiB']; | |
| const i = Math.floor(Math.log(bytes) / Math.log(threshold)); | |
| const num = bytes / Math.pow(threshold, i); | |
| return `${num.toFixed(decimals)} ${units[i]}`; | |
| const isNegative = bytes < 0; | |
| const absBytes = Math.abs(bytes); | |
| const units = si | |
| ? ['B', 'kB', 'MB', 'GB', 'TB', 'PB', 'EB', 'ZB', 'YB'] | |
| : ['B', 'KiB', 'MiB', 'GiB', 'TiB', 'PiB', 'EiB', 'ZiB', 'YiB']; | |
| const i = Math.floor(Math.log(absBytes) / Math.log(threshold)); | |
| const num = absBytes / Math.pow(threshold, i); | |
| return `${isNegative ? '-' : ''}${num.toFixed(decimals)} ${units[i]}`; |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Do negative bytes make sense? If this is called with a negative value, my assumption is that that would indicate an error, and thus isn't something we should silently handle. Let me know if you feel otherwise though!
| "@types/leaflet": "^1.9.8", | ||
| "@types/leaflet-draw": "^1.0.11", | ||
| "@types/mocha": "^9.0.0", | ||
| "@types/numeral": "^2.0.5", |
There was a problem hiding this comment.
I think this is needed because packages/compass-schema/src/components/type.tsx and packages/compass-schema/src/components/array-minichart.tsx use numeral
Co-authored-by: Copilot <[email protected]>
Addresses followups from #7381
numeralusage in compass-collection, compass-crud, and compass-indexes with centralized helpersnumeraldependency from 3 packagesChecklist
Types of changes