Skip to content
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(dataset): resizable dataset layout left column #24829

Conversation

justinpark
Copy link
Member

SUMMARY

Since #24599 introduced the TableSelector in dataset left column, it also collapses the overflowing (long) table names like before SQLLab editor.. This commit also introduces the Resizable Leftbar in dataset layout to fix the problem as SQL Lab does.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

After:

after--resizable-left-column.mov

Before:

before--resizable-column.mov

TESTING INSTRUCTIONS

Go to "Add Dataset" and choose a schema which contains a long table name

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@justinpark justinpark force-pushed the feat--resizable-dataset-layout-left-column branch from 182a087 to 7b8401a Compare July 27, 2023 23:26
@justinpark justinpark changed the title Feat resizable dataset layout left column feat(dataset): resizable dataset layout left column Jul 27, 2023
@justinpark justinpark changed the title feat(dataset): resizable dataset layout left column fix(dataset): resizable dataset layout left column Jul 28, 2023
@justinpark justinpark force-pushed the feat--resizable-dataset-layout-left-column branch from 7b8401a to 4f43aea Compare July 28, 2023 16:47
@codecov
Copy link

codecov bot commented Jul 28, 2023

Codecov Report

Merging #24829 (0697490) into master (e2d5046) will decrease coverage by 0.01%.
Report is 2 commits behind head on master.
The diff coverage is 81.81%.

❗ Current head 0697490 differs from pull request most recent head ff8a6bb. Consider uploading reports for the commit ff8a6bb to get more accurate results

@@            Coverage Diff             @@
##           master   #24829      +/-   ##
==========================================
- Coverage   68.99%   68.99%   -0.01%     
==========================================
  Files        1903     1903              
  Lines       74072    74073       +1     
  Branches     8193     8193              
==========================================
- Hits        51107    51106       -1     
  Misses      20844    20844              
- Partials     2121     2123       +2     
Flag Coverage Δ
javascript 55.82% <66.66%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
...c/features/datasets/AddDataset/LeftPanel/index.tsx 89.47% <ø> (ø)
superset-frontend/src/features/datasets/styles.ts 92.00% <50.00%> (-8.00%) ⬇️
...tend/src/features/datasets/DatasetLayout/index.tsx 100.00% <100.00%> (ø)
superset/databases/api.py 89.90% <100.00%> (ø)
superset/views/core.py 69.46% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@@ -25,22 +25,22 @@ export const StyledLayoutWrapper = styled.div`
background-color: ${({ theme }) => theme.colors.grayscale.light5};
`;

const Column = styled.div`
const Column = styled.div<{ width?: number }>`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that LeftColumn and RightColumn are pretty much overriding the properties of Column, could you remove Column? I think it will be way more readable.

export const LeftColumn = styled.div<{ theme: SupersetTheme; width?: number }>`
  width: ${({ theme, width }) => width ?? theme.gridUnit * 80}px;
  height: auto;
  flex-direction: column;
`;

export const RightColumn = styled.div`
  width: auto;
  height: auto;
  display: flex;
  flex: 1 1 auto;
  flex-direction: column;
`;

@justinpark
Copy link
Member Author

@michael-s-molina I updated the styles as you suggested. could you review this pr again?

Copy link
Member

@michael-s-molina michael-s-molina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lyndsiWilliams
Copy link
Member

/testenv up

@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2023

@lyndsiWilliams Ephemeral environment spinning up at http://54.188.162.116:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

@justinpark justinpark merged commit 6ff7fae into apache:master Aug 3, 2023
26 checks passed
@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2023

Ephemeral environment shutdown and build artifacts deleted.

@michael-s-molina michael-s-molina added the v3.0 Label added by the release manager to track PRs to be included in the 3.0 branch label Aug 3, 2023
michael-s-molina pushed a commit that referenced this pull request Aug 4, 2023
@mistercrunch mistercrunch added 🍒 3.0.0 🍒 3.0.1 🍒 3.0.2 🍒 3.0.3 🍒 3.0.4 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels labels Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/M v3.0 Label added by the release manager to track PRs to be included in the 3.0 branch 🍒 3.0.0 🍒 3.0.1 🍒 3.0.2 🍒 3.0.3 🍒 3.0.4 🚢 3.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants