feat: add import result badge and centralize status config#390
feat: add import result badge and centralize status config#390MatiasArriola wants to merge 3 commits intodevelopmentfrom
Conversation
- new ImportResultBadge component displayed after import (allowing to reopen summary modal) - refactor: extract shared computeOverallSyncStatus to domain layer - refactor: getStatusConfig to avoid code duplication, and use colors from the theme instead of hardcoding them - minor adjustaments to spacing in ImportTemplatePage
BundleMonNo change in files bundle size Groups updated (1)
Final result: ✅ View report in BundleMon website ➡️ |
There was a problem hiding this comment.
[code-only review] Thanks, @MatiasArriola! Approving :) Just some nitpick but everything looks good!
Also, just double check and remove #fuzzy in the translation files 👍
| case "NETWORK ERROR": | ||
| return { ...base, label: i18n.t("Network error") }; | ||
| case "PENDING": | ||
| return { ...base, color: colors.grey, label: i18n.t("Import pending") }; |
There was a problem hiding this comment.
[nitpick] looks like the color is already returned by getStatusConfig so overriding the color here is redundant
| OK: colors.positive, | ||
| }; | ||
|
|
||
| export function getStatusColor(status: string): string { |
There was a problem hiding this comment.
[nitpick] I think as SynchronizationStatus can be irrelevant if the status param is updated to SynchronizationStatus. Looks like the only caller is in SyncSummary.formatStatusTag which also accepts a string but can be updated to SynchronizationStatus as well
…dundant color set in getBadgeConfig
|
Thanks @gqcorneby! good catches, implemented them. I didn't even know about fuzzy on i18n 😅 |
4 participants
📌 References
📝 Implementation
ImportResultBadgecomponent displayed after import (allowing to reopen summary modal).computeOverallSyncStatusto domain layergetStatusConfigto avoid code duplication, and use colors from the theme instead of hardcoding them in each component. Now History, the new ImportResultBadge, and SyncSummary will use the same colors.ImportTemplatePage🔥 Notes for the reviewer
📹 Screenshots/Screen capture
📑 Others