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 conditionally hiding summaries #15594

Open
wants to merge 2 commits into
base: 3.x
Choose a base branch
from

Conversation

damiandejong
Copy link
Contributor

@damiandejong damiandejong commented Feb 12, 2025

Description

The ability to conditionally hide summaries as described in the documentation by injecting e.g. $query is currently not working. When injecting $query it always returns null and also the other dependencies of a summarizer cannot be reached. The reason is that they are not yet injected when the visibility condition is evaluated.

With the changes in this PR, the visibility condition is evaluated after the injection instead of beforehand.

Functional changes

  • Code style has been fixed by running the composer cs command.
  • Changes have been tested to not break existing functionality.
  • Documentation is up-to-date.

Copy link
Member

@danharrin danharrin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please look at the other usages of getSummarizers().

@danharrin danharrin added the bug Something isn't working label Feb 13, 2025
@danharrin danharrin added this to the v3 milestone Feb 13, 2025
@damiandejong
Copy link
Contributor Author

damiandejong commented Feb 14, 2025

@danharrin I have checked the other usages. Most of the other usages check whether the column actually has a summary by calling hasSummary(). I am not sure whether it's the best solution, but I got it working by passing the $query in getSummarizers() instead of injecting it at the moment of rendering the actual summarizer. See the latest commit.

There is only one usage left for which there is no evaluation of the injected parameters possible, if I understand everything correctly. When the columns are pushed to the table instance, hasSummary() is evaluated to determine whether the table has a summary overall. For each column it counts the summarizers in getSummarizers, but at that point in the lifecycle, it cannot access the the table instance yet. I am correct or do I miss something that might solve this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

3 participants