-
Notifications
You must be signed in to change notification settings - Fork 14k
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: consistent left margin for dashboard layout. #13905
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
Codecov Report
@@ Coverage Diff @@
## master #13905 +/- ##
==========================================
+ Coverage 77.55% 77.77% +0.21%
==========================================
Files 935 935
Lines 47301 47303 +2
Branches 5907 5916 +9
==========================================
+ Hits 36685 36788 +103
+ Misses 10472 10372 -100
+ Partials 144 143 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
* master: (26 commits) chore: bump to new superset-ui version (#13932) fix: do not run containers as root by default in Helm chart (#13917) feat(explore): adhoc column formatting for Table chart (#13758) fix(sqla-query): order by aggregations in Presto and Hive (#13739) feat(alert/report): add ALERTS_ATTACH_REPORTS feature flags + feature (#13894) test: Fixes PropertiesModal_spec (#13548) fix: Pin Prophet dependency after breaking changes (#13852) test: Adds tests to dnd controls (#13650) test: Adds tests to the AnnotationLayer component (#13748) test: Refactor and enhance tests for the Explore DatasourcePanel Component (#13799) Add tests (#13778) test: DisplayQueryButton (#13750) Fixing condition around left margin for dashboard layout. Fixes #13863 (#13905) Revert "fix: select table overlay (#13694)" (#13901) test: Adds tests to the OptionControls component (#13729) test: DatasourceControl (#13605) tests for function handleScroll (#13896) test: Adds tests to the CustomFrame component (#13675) test: Adds tests to the AdvancedFrame component (#13664) test: DataTableControl (#13668) ...
🏷️ 2021.13 |
…e#13863 (apache#13905) (cherry picked from commit a7f48c6)
SUMMARY
Oops! Erroneously merged this to a feature branch instead of master. Ugh.
See #13884 for a history/comments, and prior approvals.
A recent PR, #13723, made some changes around how the sidebar works for the native filter features. This looks great when the feature flag is on, but when the feature flag is off, we lose the left margin, as reported in #13863.
This PR makes sure the styling is consistent, by considering feature flag setting, and only toggling off
dashboardFiltersOpen
(which sets the margin) when the feature is indeed enabled.@simcha90 if you have a more elegant way to tackle this, or ideas for sufficient testing to cover this possible regression, please let me know your thoughts.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
BEFORE (
DASHBOARD_NATIVE_FILTERS
set to False):AFTER (
DASHBOARD_NATIVE_FILTERS
set to False):AFTER (
DASHBOARD_NATIVE_FILTERS
set to True):TEST PLAN
Verified that the margin is restored, in view and edit mode, with the
DASHBOARD_NATIVE_FILTERS
flag on and off.ADDITIONAL INFORMATION