fix(embedded): respect show_filters URL param in standalone report mode - #42307
Open
rusackas wants to merge 1 commit into
Open
fix(embedded): respect show_filters URL param in standalone report mode#42307rusackas wants to merge 1 commit into
rusackas wants to merge 1 commit into
Conversation
Report mode (standalone=3) unconditionally hides the filter bar via
hidden={isReport}, since it's meant for one-shot screenshot renders
(reports, thumbnails). Embedded SDK consumers also use standalone=3 to
hide the title/tab/nav (hideTitle + hideTab both map into that bitmask),
but may still want the filter bar visible via
dashboardUiConfig.filters.visible, which maps to the show_filters URL
param. That param was declared in URL_PARAMS but never actually wired
up, so it was silently ignored, breaking embedded dashboards that
wanted filters visible while hiding chrome.
hideFilterBar now only forces the filter bar hidden in report mode when
show_filters isn't explicitly set to true, so report-mode defaults are
unchanged but embedded consumers can override them.
Fixes #30630
Contributor
Code Review Agent Run #2fa95bActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
Contributor
|
@sadpandajoe This PR is ready for review. Please review when you have a chance. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
SUMMARY
Report mode (
standalone=3) unconditionally hides the filter bar viahidden={isReport}on both the horizontalFilterBarand verticalFiltersPanel. That's correct for its original purpose (one-shot screenshot renders for scheduled reports/thumbnails), but the Embedded SDK also usesstandalone=3to hide the title/tabs/nav (itshideTitle+hideTabconfig both fold into that same bitmask), and embedded consumers can still ask for filters viadashboardUiConfig.filters.visible, which maps to theshow_filtersURL param.That param was declared in
URL_PARAMSbut never actually wired intoDashboardBuilder, so it was silently ignored — the filter bar stayed forced-hidden any timestandalone=3was in play, regardless ofshow_filters.This introduces
hideFilterBar = isReport && showFiltersUrlParam !== trueso:show_filters=trueexplicitly overrides the report-mode hidingshow_filters=false(or unset) keeps the existing hidden behaviorBEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — this only changes a CSS
display: nonetoggle; behavior is covered by unit tests.TESTING INSTRUCTIONS
?standalone=3— filter bar should be hidden (unchanged).?standalone=3&show_filters=true— filter bar should now be visible.?standalone=3&show_filters=false— filter bar should still be hidden.npm run test -- DashboardBuilder.test.tsxinsuperset-frontend.ADDITIONAL INFORMATION