Feature/expandable company rows #503
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Greenheart
commented
May 7, 2024
| ` | ||
|
|
||
| type TableProps<T extends object> = { | ||
| data: T[] | ||
| columns: ColumnDef<T>[] | ||
| routeString?: string | ||
| // IDEA: It might be better to turn ComparisionTable into two specific components, one for every use case | ||
| dataType?: 'municipalities' | 'companies' | ||
| renderSubComponent?: ({ row }: { row: Row<T> }) => JSX.Element |
There was a problem hiding this comment.
This is a quick and dirty way to allow custom rendering logic for companies and municipalities. However, it might be worth having two separate tables instead, to reduce conditional logic and the need for testing as many edge cases.
In other words, some duplication for cleaner, focused table components.
Worth doing? Or should we leave it as it is for now?
There was a problem hiding this comment.
Have it on our todo and see if there's time before Tue? (and otherwise fix after release)
There was a problem hiding this comment.
Sounds good. Adding to the list in #430
elvbom
reviewed
May 7, 2024
| ` | ||
|
|
||
| type TableProps<T extends object> = { | ||
| data: T[] | ||
| columns: ColumnDef<T>[] | ||
| routeString?: string | ||
| // IDEA: It might be better to turn ComparisionTable into two specific components, one for every use case | ||
| dataType?: 'municipalities' | 'companies' | ||
| renderSubComponent?: ({ row }: { row: Row<T> }) => JSX.Element |
There was a problem hiding this comment.
Have it on our todo and see if there's time before Tue? (and otherwise fix after release)
Greenheart
added a commit
that referenced
this pull request
May 13, 2024
* add new components for regional and company views * add company data service and dito create list * enable different routes when row in comparisontable is clicked * minor tweaks * center switch text * company list with name, scope1n2, scope 3 and url in place * fix lint * styling and cleanup * minor styling * add todo * minor fix * Use Swedish number formatting. Types need to be fixed. * Adapt RegionalView to work with i18n * Ignore typescript error which prevents preview builds * Add idea * routing on pill switch * toggle between geo datasets * make company and list defaults * make pill switch toggle set dataset and view to default * rm unnecessary file * minor cleanup * make pillswitch wai-aria complient and keyboard accessible (#501) * Feat/garbo updated company data (#502) * updated company data * rm leading and trailing spaces * Fix formatting for emission scopes that are not reported --------- Co-authored-by: Samuel Plumppu <6125097+Greenheart@users.noreply.github.com> * Feature/expandable company rows (#503) * Fix React warning and document upcoming tasks * WIP: expandable rows * Make every row expandable by default * Fix syntax error * Clarify styling for expanded rows * Show company name in collapsed view * Improve styling for expanded rows * Adapt for tablet * Remove old code * Prevent column widths from changing when expanding rows * Remove trim * Use translated string * Remove link from collapsed table row (2nd time due to merge resolution gone wrong) * Use name property * Make clickable areas more obvious for expanded rows * Prevent visual glitch for sticky table header. Also simplify styling * Only apply column sizing hack to company table * Improve table header spacing and font size for smallest screens * Add compact layout for PageWrapper component to allow full width layouts for smallest screens * Break long names to prevent overflow * Use consistent locale for Intl.NumberFormat * Replace with proper null value to know when data is missing. This helps avoid a JS edge case with `Number.isNaN('n.a')` which one would expect to be `false` but in fact is `true` for some reason. * Fix sorting for emission scopes * Revert "Merge branch 'staging' into feat/garbo-mvp-w-routing" This reverts commit 5eb2a94, reversing changes made to 7566e42. revert merge * Update company parsing script to prefer null values. Also minify company output data to improve performance. * add hardcoded redirects to old paths * Fix bug introduced with merge * Add privacy policy link (#546) * Feature: Add expand icon to comparison table to improve UX (#542) * Example arrow for toggleable section * Rename icons to better represent their intent * Reduce font sizes for small screens to prevent overflow. Also adjust column width to better reflect data. * Expandable icons alternative 1: +/- * Expandable icons alternative 2: rotating arrow * Feature: Add sorting icons for table columns to make it easier to know you can sort change the sorting (#543) * Show arrow icons to indicate that table columns can be sorted * Cleanup * Improve layout for table header with sorting icons. * Cleanup * Fix: Update StartPage tests to handle both CompanyView and RegionalView (#541) * Separate translation keys for regionalView vs companyView * Speed up tests by replacing jest-dom with vitest assertions * Use ESM for vitest * Update testing related deps * Update tests for RegionalView and CompanyView * Re-introduce @testing-library/jest-dom and upgrade react to ^18.3.1 to resolve peerDependency error * Fix subscript formatting in README.md * Add editing info and link to sources and method to table footer in the CompanyView (#539) * Add info to table footer * Update copy to reflect latest changes * Update copy to match latest suggestion * Bugfix: Ensure state from URL can control which dataGroup to show by default. This allows linking to the RegionalView again. * Simplify state management for dataset/dataview routing and prevent re-renders. This greatly improves performance and UX. * Redirect to default dataset or dataView instead of showing 404 page. * Make caching include both dataGroup and dataset. Simplify PillSwitch to reduce client side JS and rely on server state + link prefetching instead. * Fix broken caching for map API endpoint * Use shared constant for default cache time * Fix hydration warning * Use correct translation function * Add caching for municipalities and companies since they are the heavy datasets. Remove response caching since that duplicates data and uses more RAM. * Ensure the selected dataset and dataview is reflected in the URL for regional data * update company names and data (#554) * Improve layout for ComparisonTable, update copy, fix tests (#556) * Add missing key prop * Remove copy from CompanyView to match latest suggestion * Update table headers and add some strings to translations * Improve table header font size for small screen sizes * Improve size and spacing for table * Use full-width table for all mobile sizes. * Fix offset for sticky table header. Also hide background content above it. * Improve compact layout for tables on smaller screens * Improve table layout for small screens. * More compact table header to save screen height on mobile * Cover full width with table for all mobile devices * Fix margin positioning for ToggleButton * Update table footer copy * Reference arrow svg as img instead of inlined SVG. This improves rendering performance. * Update newsletter info * Update about us text * Move text about previous project * Update sources and method * Use plural key to prepare for i18n * Fix tests for StartPage by splitting into two modules: CompanyView and RegionalView * Cleanup tests * Add privacy policy as separate page * Add description of the method for company data * reintroduce 12px padding to sides of table (#558) --------- Co-authored-by: Samuel Plumppu <6125097+Greenheart@users.noreply.github.com>
Greenheart
added a commit
that referenced
this pull request
May 13, 2024
* add new components for regional and company views * add company data service and dito create list * enable different routes when row in comparisontable is clicked * minor tweaks * center switch text * company list with name, scope1n2, scope 3 and url in place * fix lint * styling and cleanup * minor styling * add todo * minor fix * Use Swedish number formatting. Types need to be fixed. * Adapt RegionalView to work with i18n * Ignore typescript error which prevents preview builds * Add idea * routing on pill switch * toggle between geo datasets * make company and list defaults * make pill switch toggle set dataset and view to default * rm unnecessary file * minor cleanup * make pillswitch wai-aria complient and keyboard accessible (#501) * Feat/garbo updated company data (#502) * updated company data * rm leading and trailing spaces * Fix formatting for emission scopes that are not reported --------- Co-authored-by: Samuel Plumppu <6125097+Greenheart@users.noreply.github.com> * Feature/expandable company rows (#503) * Fix React warning and document upcoming tasks * WIP: expandable rows * Make every row expandable by default * Fix syntax error * Clarify styling for expanded rows * Show company name in collapsed view * Improve styling for expanded rows * Adapt for tablet * Remove old code * Prevent column widths from changing when expanding rows * Remove trim * Use translated string * Remove link from collapsed table row (2nd time due to merge resolution gone wrong) * Use name property * Make clickable areas more obvious for expanded rows * Prevent visual glitch for sticky table header. Also simplify styling * Only apply column sizing hack to company table * Improve table header spacing and font size for smallest screens * Add compact layout for PageWrapper component to allow full width layouts for smallest screens * Break long names to prevent overflow * Use consistent locale for Intl.NumberFormat * Replace with proper null value to know when data is missing. This helps avoid a JS edge case with `Number.isNaN('n.a')` which one would expect to be `false` but in fact is `true` for some reason. * Fix sorting for emission scopes * Revert "Merge branch 'staging' into feat/garbo-mvp-w-routing" This reverts commit 5eb2a94, reversing changes made to 7566e42. revert merge * Update company parsing script to prefer null values. Also minify company output data to improve performance. * add hardcoded redirects to old paths * Fix bug introduced with merge * Add privacy policy link (#546) * Feature: Add expand icon to comparison table to improve UX (#542) * Example arrow for toggleable section * Rename icons to better represent their intent * Reduce font sizes for small screens to prevent overflow. Also adjust column width to better reflect data. * Expandable icons alternative 1: +/- * Expandable icons alternative 2: rotating arrow * Feature: Add sorting icons for table columns to make it easier to know you can sort change the sorting (#543) * Show arrow icons to indicate that table columns can be sorted * Cleanup * Improve layout for table header with sorting icons. * Cleanup * Fix: Update StartPage tests to handle both CompanyView and RegionalView (#541) * Separate translation keys for regionalView vs companyView * Speed up tests by replacing jest-dom with vitest assertions * Use ESM for vitest * Update testing related deps * Update tests for RegionalView and CompanyView * Re-introduce @testing-library/jest-dom and upgrade react to ^18.3.1 to resolve peerDependency error * Fix subscript formatting in README.md * Add editing info and link to sources and method to table footer in the CompanyView (#539) * Add info to table footer * Update copy to reflect latest changes * Update copy to match latest suggestion * Bugfix: Ensure state from URL can control which dataGroup to show by default. This allows linking to the RegionalView again. * Simplify state management for dataset/dataview routing and prevent re-renders. This greatly improves performance and UX. * Redirect to default dataset or dataView instead of showing 404 page. * Make caching include both dataGroup and dataset. Simplify PillSwitch to reduce client side JS and rely on server state + link prefetching instead. * Fix broken caching for map API endpoint * Use shared constant for default cache time * Fix hydration warning * Use correct translation function * Add caching for municipalities and companies since they are the heavy datasets. Remove response caching since that duplicates data and uses more RAM. * Ensure the selected dataset and dataview is reflected in the URL for regional data * update company names and data (#554) * Improve layout for ComparisonTable, update copy, fix tests (#556) * Add missing key prop * Remove copy from CompanyView to match latest suggestion * Update table headers and add some strings to translations * Improve table header font size for small screen sizes * Improve size and spacing for table * Use full-width table for all mobile sizes. * Fix offset for sticky table header. Also hide background content above it. * Improve compact layout for tables on smaller screens * Improve table layout for small screens. * More compact table header to save screen height on mobile * Cover full width with table for all mobile devices * Fix margin positioning for ToggleButton * Update table footer copy * Reference arrow svg as img instead of inlined SVG. This improves rendering performance. * Update newsletter info * Update about us text * Move text about previous project * Update sources and method * Use plural key to prepare for i18n * Fix tests for StartPage by splitting into two modules: CompanyView and RegionalView * Cleanup tests * Add privacy policy as separate page * Add description of the method for company data * reintroduce 12px padding to sides of table (#558) * update company emissions data (#560) * Revert "Merge branch 'main' into staging" This reverts commit 3b83e9d, reversing changes made to 274943e. --------- Co-authored-by: Samuel Plumppu <6125097+Greenheart@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.