Skip to content
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

[14.0][IMP] use cell_style_props for val_rendered in kpi matrix #561

Open
wants to merge 1 commit into
base: 14.0
Choose a base branch
from

Conversation

jonasbuchholz
Copy link

So far, the values set in set_values_detail_account were rendered using the row style properties, before evaluating the row's style expression. Thus, even though the correct style properties are passed as cell_style_props, some changes by the style_expression (e.g. prefix, suffix, ...) would not take effect.
With these changes, the style_expression gets evaluated before rendering the value, and the correct cell_style_props are passed to style_model.render.

@OCA-git-bot
Copy link
Contributor

Hi @sbidoul,
some modules you are maintaining are being modified, check this out!

@sbidoul
Copy link
Member

sbidoul commented Apr 14, 2023

Hi @jonasbuchholz thanks for the contrib.

Can you post a few screenshot with a sample config to help me understand what this fixes ?

@jonasbuchholz
Copy link
Author

Hi @sbidoul , thank you for your swift response!
Sure! One example would be a KPI that uses different rounding based on its value:
demo_kpi
The style that is selected for the template in this case is "No Rounding". Without these changes, the rounding property is never applied to the values because the style_expression gets evaluated after they have been rendered:
without_fix
If the rendering happens after evaluating the style_expression:
with_fix

I hope this helps to explain what these changes do.

@github-actions
Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Aug 13, 2023
@sbidoul sbidoul added bugfix no stale Use this label to prevent the automated stale action from closing this PR/Issue. and removed stale PR/Issue without recent activity, it'll be soon closed automatically. labels Aug 21, 2023
@sbidoul
Copy link
Member

sbidoul commented Aug 21, 2023

Thanks the change makes sense to me. I've not had time to look into it yet. Wondering how hard it would be to add a test for this.

@sbidoul sbidoul added the 14.0 label Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
14.0 bugfix no stale Use this label to prevent the automated stale action from closing this PR/Issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants