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

Fix Issue with hidden aggregate fields #790

Merged
merged 3 commits into from
Nov 19, 2022
Merged

Conversation

limitednl
Copy link
Contributor

Added the option to keep showing aggregate fields on attributes even when the query is not aggregating.
See: #789

@rappen
Copy link
Owner

rappen commented Oct 5, 2022

Thanks for this PR!

However, I think you are missing a "not" somewhere...
If I keep the settings default (Hide fields when aggregating is set) it looks like normal for the attributes.
If I uncheck this new setting, aggregate-related properties are never shown.

Are you sure about this feature?

- text change
- fixed logic
@limitednl
Copy link
Contributor Author

You are right! The indended logic was not what was implemented. I fixed it in the seccond commit.

Thb, ive not tested it because i dont know how to set up this project in xrmtoolbox and only made the PR to make a starting point for this feature/fix for you to continue on. I do not know what patterns you use and if the solution i suggested is the way you would have implemented it.
I hope it is of some use for you.

@rappen
Copy link
Owner

rappen commented Oct 5, 2022

Maybe this can help you on with debug your developments?
https://www.xrmtoolbox.com/documentation/for-developers/debug/

@limitednl
Copy link
Contributor Author

Thanks for letting me know how to debug the code. For me the code works as intended now. It is up to you to decide what to do with it

@rappen rappen merged commit 908901b into rappen:master Nov 19, 2022
rappen added a commit that referenced this pull request Nov 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants