-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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: keep the tab order #30888
base: master
Are you sure you want to change the base?
fix: keep the tab order #30888
Conversation
FYI @yousoph |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #30888 +/- ##
===========================================
+ Coverage 60.48% 83.46% +22.97%
===========================================
Files 1931 546 -1385
Lines 76236 39291 -36945
Branches 8568 0 -8568
===========================================
- Hits 46114 32794 -13320
+ Misses 28017 6497 -21520
+ Partials 2105 0 -2105
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
LGTM, but @eschutho might be better poised to actually test this before approving/merging. |
superset/dashboards/api.py
Outdated
response_data = json.dumps({"result": result}) | ||
return Response( | ||
response_data, status=HTTPStatus.OK, mimetype="application/json" | ||
) |
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.
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.
superset/models/dashboard.py
Outdated
if (childs := node.get("children")) and node["type"] == "TABS": | ||
tabs_order.extend(childs) | ||
if tabs_order: | ||
all_tabs = {str(t): all_tabs[str(t)] for t in tabs_order} |
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.
@US579 Since the api (in addition to the dictionary for all_tabs) returns a nested array structure of the tabs which would maintain their order, can you iterate through the tab_tree instead of relying on the all_tabs dict?
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.
ah yep good found!
3d2d8a6
to
eceb68d
Compare
8a0f4a1
to
61d4830
Compare
61d4830
to
926b9a4
Compare
SUMMARY
When export all tabs to a PDF in report & alert, user noticed that the all tabs export doesn't seem to export the tabs in any specific order (our report with 3 tabs exported with tab 2, then 3, then 1 in the PDF)
reference: #30694 (comment)
Context
The exported tabs are now ordered based on the sequence of anchors under the all_tabs field from the tabs endpoint response. This enhancement aligns the export order with the order displayed on the dashboard.
BEFORE PDF
before.pdf
AFTER PDF
after.pdf
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION