Add pivot clockwise to navigation drawer edit pane#714
Merged
MikaelNordberg merged 17 commits intomainfrom Jul 1, 2025
Merged
Conversation
Branch Deployment to Cloudflare Pages🚛 Preview URL: https://9fccdc2e.pxweb2.pages.dev |
Adds a dynamic title based on the current selection, to the ContentTop
Refactor to minimize the rerendering of the Table component, either when loading data or pivoting it. When testing the pivot on a large Table (close to 150000 cells), it could take 4+ seconds. This should significantly improve that by memoizing more, and also by moving the Navigation Drawer's views into seperate components. Did not move the VariableList yet. Co-authored-by: Mikael Nordberg <54406826+MikaelNordberg@users.noreply.github.com>
Since we want to minimize the time it takes to render tables, we should only do the formatting with react-i18next when needed. And we should cache the translation/formatting. This expands on the formatting, so we now can cache after the first time. During some testing the time to format the values in the table could take 300+ ms, and this caching removes that step in the table rendering after the first time the value is shown. Also added tests.
|
SjurSutterudSagen
changed the title
SjurSutterudSagen
requested review from
KentMossback,
MikaelNordberg,
PerIngeVaaje,
likp,
michaelpande,
oyessb and
runejo
as code owners
MikaelNordberg
approved these changes
Jul 1, 2025
Contributor
MikaelNordberg
left a comment
MikaelNordberg
left a comment
There was a problem hiding this comment.
Looks good.
A possible improvement could be to set the formatted data cell value already in the mapping from jsonstat2.
MikaelNordberg
deleted the
feature/PxWeb2-688-pivot-clockwise-in-navigationDrawer
branch
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This adds a button to pivot the table head and stub. Reused the function that was made some time ago. The pivot function itself has not been updated. This also means we need to add the dynamic title to ContentTop, which is based on the order of the variables in the stub and heading.
As part of this we noticed some performance issue, and several components and functions have been memoized. Additionally we saw that a major bottleneck was the formatting that was done on the values in the cells, and this formatting has been moved to the fetchFromApi function, which means it only happens once; when a value is loaded into the cube. Used this guide to using react-i18next outside of React to do the formatting: https://www.locize.com/blog/how-to-use-i18next-t-outside-react-components
During testing now, it seems like a deployed version uses about 1.5s to pivot a table with over 130,000 cells.
Also added support for 6 decimals. But we might want to have another look at reducing the number of formatting strings later, maybe we could use plurals for them?