You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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
Views now accept external variables that merge with URL parameters, providing enhanced flexibility in variable management and enabling more sophisticated configuration options across view sections and components. This improvement allows for better control over view behavior and data handling.
✏️ Tip: You can customize this high-level summary in your review settings.
This PR extends ViewSection and ViewWithSections components to accept and propagate external variables as props, enabling variable merging with runtime URL-based parameters throughout the view component hierarchy.
Changes
Cohort / File(s)
Change Summary
Variables propagation in view components src/pages/views/components/ViewSection.tsx, src/pages/views/components/ViewWithSections.tsx
ViewSection now accepts a variables prop (Record<string, string>) and merges it with URL-derived parameters using memoized values; ViewWithSections propagates merged currentVariables to ViewSection instances in both primary and mapped section contexts
feat: view sections #2739: Introduces ViewSection with variable aggregation logic; this PR extends that component with external variable merging capabilities
Suggested reviewers
moshloop
Pre-merge checks and finishing touches
✅ Passed checks (3 passed)
Check name
Status
Explanation
Description Check
✅ Passed
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
✅ Passed
The title directly describes the main change: adding missing variables prop to the ViewSection component for variable passing.
Docstring Coverage
✅ Passed
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
📝 Generate docstrings
🧪 Generate unit tests (beta)
Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix/missing-variables-in-view-section
📜 Recent review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 0ae198b and 62cf330.
56-63: Wire currentVariables through all ViewSection instances
Forwarding currentVariables via variables={currentVariables} to both the primary ViewSection and each mapped section keeps all sections aligned with the parent view’s current variable set and matches the new ViewSectionProps.variables API. This should address the missing-variables issue without changing visibility behavior (hideVariables) in nested views.
Also applies to: 67-78
src/pages/views/components/ViewSection.tsx (1)
12-22: Variable merge and propagation in ViewSection look consistent
Using the new variables prop (aliased to defaultVariables) and building currentViewVariables by spreading defaultVariables first and URL-derived paramVariables second gives URL params precedence over defaults, then reuses that merged object in both the fetch and the nested <View>’s currentVariables prop. This is a clean way to inject parent defaults while still allowing runtime overrides, and it should fix the missing-variables behavior for sections.
Can you confirm that “URL overrides defaults” is the intended precedence for all callers, including aggregator views?
Also applies to: 30-37, 45-48, 140-142
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
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.
fixes regression introduced in : #2766
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.