-
Notifications
You must be signed in to change notification settings - Fork 68
[IMP] pivots: implement pivot table styles #7455
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
e30e965 to
cd9fa6d
Compare
| fillColor?: Color; | ||
| textColor?: Color; | ||
| fontSize?: number; // in pt, not in px! | ||
| skipCellGridLines?: boolean; |
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'm really not a fan of adding this to Style which is supposed to be the core, raw data for style, while skipCellGridLines seems to be something derived from other things
EDIT: mouais...I see
packages/o-spreadsheet-engine/src/helpers/pivot_table_presets.ts
Outdated
Show resolved
Hide resolved
packages/o-spreadsheet-engine/src/plugins/ui_core_views/pivot_ui.ts
Outdated
Show resolved
Hide resolved
| templateName: "pivotMediumBlackHeaders", | ||
| primaryColor: colorSet.highlight, | ||
| wholeTable: { | ||
| style: { skipCellGridLines: true, fillColor: colorSet.light }, |
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.
skipCellGridLines: true not useful if there's a fill color ?
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.
But drawing grid lines just to paint a color over it is also useless 🤷
|
And the feature is really cool 👍 |
This commit extracts the logic that tracks the occurrence of the SUBTOTAL formulas to a more generic plugin named `formula_tracker`. This will be used to track the occurrences of `PIVOT` formulas in a future commit. Task: 4552232
Before this commit, the table style presets contained all the style of a table, except the `bold` property, which was handled separately and applied to every total/header row or first/last column. This made the code harder to read with two sources of truth for the style, and with the introduction of pivot table styles, every header row won't be in bold anymore. This commit adds the `bold` property to the table style presets, os it is handled like every other style property. Task: 4552232
cb9ce4a to
ed35656
Compare
This commit introducing cell animations added an option `skipCellGridLines` to the rendering boxes. The option allows to skip drawing grid lines for specific cells. This commit changes this rendering box option into a standard cell style option, so it can be used in other parts of the codebase as well. Task: 4552232
With this commit, the cell background are now drawn totally covering the grid lines. It's way prettier for table/pivots to have an uniform background color without grid lines breaking it. The downside is we cannot draw the hover overlay with a color alpha, because it would overlap with the next cell, making a darker color at the border. We can workaround by merging the colors programmatically and avoid drawing with alpha. Task: 4552232
With this commit, we don't need to manually create a dynamic table to a pivot to have a style applied. Instead we can add a style in the pivot definition, and dynamic tables will automatically be created on the dynamic pivot formulas. Those new pivot styles are better than traditional tables styles because: - they are directly linked to the pivot, taking into account the number of headers, the presence of totals, etc. - they are automatically added on `=PIVOT()` formulas, without the need to create a dynamic table first. - they are more powerful than the old table styles, they can have a style for the sub-headers, the measure headers, etc. Task: 4552232
ed35656 to
63cb270
Compare

Description:
[IMP] pivots: implement pivot table styles
With this commit, we don't need to manually create a dynamic table
to a pivot to have a style applied. Instead we can add a style in the
pivot definition, and dynamic tables will automatically be created on
the dynamic pivot formulas.
Those new pivot styles are better than traditional tables styles because:
number of headers, the presence of totals, etc.
=PIVOT()formulas, without the need tocreate a dynamic table first.
style for the sub-headers, the measure headers, etc.
[IMP] renderer: draw cell background over grid lines
With this commit, the cell background are now drawn totally covering
the grid lines. It's way prettier for table/pivots to have an uniform
background color without grid lines breaking it.
The downside is we cannot draw the hover overlay with a color alpha,
because it would overlap with the next cell, making a darker color
at the border. We can workaround by merging the colors programmatically
and avoid drawing with alpha.
[IMP] style: add
skipCellGridLinesstyle optionThis commit introducing cell animations added an option
skipCellGridLinesto the rendering boxes. The option allows toskip drawing grid lines for specific cells.
This commit changes this rendering box option into a standard cell
style option, so it can be used in other parts of the codebase as well.
[REF] table style: add
boldto table style presetsBefore this commit, the table style presets contained all the style of
a table, except the
boldproperty, which was handled separately andapplied to every total/header row or first/last column.
This made the code harder to read with two sources of truth for the
style, and with the introduction of pivot table styles, every header
row won't be in bold anymore.
This commit adds the
boldproperty to the table style presets,os it is handled like every other style property.
Task: 4552232
review checklist