-
Notifications
You must be signed in to change notification settings - Fork 45
Feature/COR-1263-table-consistency-implementations #4614
Feature/COR-1263-table-consistency-implementations #4614
Conversation
… WIP. Co-authored-by: VWSCoronaDashboard26<[email protected]>
… advice table. Rename some stuff. Extract types to its own file. Co-authored-by: Ros, Luuk <[email protected]>
…ur per age group table.
…g, refactoring, etc.
…orrect typing and some abstraction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I salute the elegant solution and the amazing clean up that followed. Great stuff! 🙌🏻
Still some minor comments and a TODO
left, but other than that it looks really good.
packages/app/src/components/tables/components/wide-percentage.tsx
Outdated
Show resolved
Hide resolved
packages/app/src/domain/behavior/components/behavior-icon-with-label.tsx
Outdated
Show resolved
Hide resolved
packages/app/src/domain/variants/variants-table-tile/components/percentage-bar-with-number.tsx
Outdated
Show resolved
Hide resolved
packages/app/src/domain/variants/variants-table-tile/components/shared-table-components.tsx
Outdated
Show resolved
Hide resolved
…s, adding sanity key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@VWSCoronaDashboard26 and @VWSCoronaDashboard18 I have applied the changes required from your feedback. Some comments which were outdated at the time I went through them were already marked as resolved. I've left the rest unresolved for you to resolve them yourselves if you are happy with the changes.
packages/app/src/domain/variants/variants-table-tile/components/percentage-bar-with-number.tsx
Outdated
Show resolved
Hide resolved
packages/app/src/domain/behavior/components/behavior-icon-with-label.tsx
Outdated
Show resolved
Hide resolved
packages/app/src/components/tables/components/percentage-bar-without-number.tsx
Outdated
Show resolved
Hide resolved
…or the percentage bar column to all relevant tables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pre-approving, but one very minor comment has been left as unresolved as it might've been forgotten.
Summary
The motivation behind this ticket was to make the tables across the site consistent in terms of design. It was taken one step further by refactoring/removing some components so that there is reduced duplicity of components. This results in quite a few changes, but should help in the long run.
The changes impact the following pages:
The changes impact all the tables on those pages. 2 on the behaviour pages and 3 on the vaccination pages.
Screenshots
NOTE: For the Vaccinations page you see the before and after of only one Vaccine Coverage table. The 2 others are in the archived section and have been made to be exactly the same as the non-archived one. Additionally, in the mobile/narrow screenshots, some of the tables might be cut off due to a lack of vertical screen space when taking a screenshot.
Before - Desktop
___ ___ ___Before - Mobile
--- --- ---After - Desktop
--- --- ---After - Mobile
--- --- ---