refactor: Boostrap to AntD - Tabs #14048
Merged
Conversation
This file contains 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
michael-s-molina
commented
Apr 9, 2021
| }; | ||
|
|
||
| const DashboardContainer: FC<DashboardContainerProps> = ({ | ||
| topLevelTabs, | ||
| handleChangeTab, |
There was a problem hiding this comment.
handleChangeTab was never called during runtime and the function interfaces were not compatible:
TabContainer.onSelect does not provide pathToTabIndex to (event: SyntheticEvent<TabContainer, Event>) => void
michael-s-molina
commented
Apr 9, 2021
| // lets us keep the same React component tree when !!topLevelTabs changes. | ||
| // This avoids expensive mounts/unmounts of the entire dashboard. | ||
| <Tabs.TabPane | ||
| key={index === 0 ? DASHBOARD_GRID_ID : index.toString()} |
There was a problem hiding this comment.
Tabs.TabPane uses the key as the eventKey
michael-s-molina
commented
Apr 9, 2021
| @@ -58,6 +54,9 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ | |||
| ? topLevelTabs.children | |||
| : [DASHBOARD_GRID_ID]; | |||
|
|
|||
| const min = Math.min(tabIndex, childIds.length - 1); | |||
| const activeKey = min === 0 ? DASHBOARD_GRID_ID : min.toString(); | |||
There was a problem hiding this comment.
Ensures that the key of the first element is always DASHBOARD_GRID_ID to avoid expensive mounts/unmounts of the entire dashboard.
michael-s-molina
force-pushed
the
bootstrap-to-antd-tabs
branch
from
5cd628d
to
01722fe
Compare
Codecov Report
@@ Coverage Diff @@
## master #14048 +/- ##
==========================================
- Coverage 77.15% 77.04% -0.11%
==========================================
Files 954 946 -8
Lines 48189 47987 -202
Branches 6002 5963 -39
==========================================
- Hits 37180 36972 -208
- Misses 10812 10814 +2
- Partials 197 201 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
michael-s-molina
force-pushed
the
bootstrap-to-antd-tabs
branch
from
01722fe
to
9c7f8ab
Compare
michael-s-molina
force-pushed
the
bootstrap-to-antd-tabs
branch
3 times, most recently
from
e64b4fa
to
f5e1f00
Compare
geido
reviewed
Apr 20, 2021
superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx
Show resolved
Hide resolved
michael-s-molina
force-pushed
the
bootstrap-to-antd-tabs
branch
from
f5e1f00
to
6dd3538
Compare
|
/testenv up |
|
@pkdotson Ephemeral environment spinning up at http://18.236.152.187:8080. Credentials are |
pkdotson
approved these changes
Apr 27, 2021
michael-s-molina
force-pushed
the
bootstrap-to-antd-tabs
branch
from
6dd3538
to
ccec4ab
Compare
michael-s-molina
force-pushed
the
bootstrap-to-antd-tabs
branch
from
ccec4ab
to
2480a50
Compare
villebro
approved these changes
Apr 29, 2021
|
Ephemeral environment shutdown and build artifacts deleted. |
amitmiran137
pushed a commit
to nielsen-oss/superset
that referenced
this pull request
May 3, 2021
* master: (38 commits) refactor(native-filters): allow cascading only for filter_select (apache#14441) test(maximize-chart): Add tests to maximize chart action (apache#14371) fix: fixing mysql error message (apache#14416) feat: Logic added to limiting factor column in Query model (apache#13521) change relationship (apache#14435) fix: bootstrap data permissions (apache#14348) fix: parse simple string error message values (apache#14360) chore: add stack trace to all calls of logger.error (apache#14382) update README with new docs and recordings (apache#14432) Renamed impyla from implya in impala.mdx and Renamed PIP package impyla from impala in index.mdx (apache#14425) fix(native-filters): fix filter scope error (apache#14426) feat: Adding limiting_factor column to Query model (apache#14234) feat: Add etag caching to dashboard APIs (apache#14357) chore: Moves Card to the components folder (apache#14139) feat: Dynamic imports for the Icons component (apache#14318) feat: Support env vars configuration for WebSocket server (apache#14398) fix: SQLLab role permissions (apache#14372) fix(native-filters): always show filters without dataset (apache#14409) fix error getting partitionQuery from table.partition (apache#14369) refactor: Boostrap to AntD - Tabs (apache#14048) ...
QAlexBall
pushed a commit
to QAlexBall/superset
that referenced
this pull request
Dec 29, 2021
mistercrunch
added
🏷️ bot
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
TabContainer,TabPanelandTabContentcomponents from Bootstrap to AntDSee: #10254
@rusackas @junlincc @pkdotson
@villebro I'm tagging you also because of the changes in
DashboardContainer. Can you help with the review? I'll add some comments to the code to provide more context.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
These are the impacted screens. No perceived difference after the changes.
TEST PLAN
1 - Access different dashboard layouts, with different tab levels
2 - Check that all layouts are working
ADDITIONAL INFORMATION