fix: adjusted tab height #13822

AAfghahi · 2021-03-26T20:12:19Z

SUMMARY

The scroll bar on query history disappeared based on a redesign that we did for the results page to add rows returned.

I adjusted the tab height of rows returned, in order to not have a scroll appear there, and added the scroll back to query history.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

after:

TEST PLAN

Manually checked this through docker to make sure that the portions that invoke the component work as expected.

ADDITIONAL INFORMATION

codecov · 2021-03-26T20:29:10Z

Codecov Report

Merging #13822 (d5ed0be) into master (db1d598) will increase coverage by 1.52%.
The diff coverage is 83.21%.

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

@@            Coverage Diff             @@
##           master   #13822      +/-   ##
==========================================
+ Coverage   76.81%   78.33%   +1.52%     
==========================================
  Files         934      934              
  Lines       47249    47350     +101     
  Branches     5892     5948      +56     
==========================================
+ Hits        36294    37092     +798     
+ Misses      10808    10114     -694     
+ Partials      147      144       -3

Flag	Coverage Δ
cypress	`56.03% <64.77%> (+2.94%)`	⬆️
javascript	`66.28% <63.82%> (+2.54%)`	⬆️

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

Impacted Files	Coverage Δ
...tend/src/SqlLab/components/ScheduleQueryButton.tsx	`28.12% <ø> (ø)`
...frontend/src/components/Checkbox/CheckboxIcons.tsx	`85.71% <ø> (ø)`
...frontend/src/components/DatabaseSelector/index.tsx	`92.70% <ø> (+1.04%)`	⬆️
superset-frontend/src/components/Icon/index.tsx	`100.00% <ø> (ø)`
superset-frontend/src/components/Loading/index.tsx	`100.00% <ø> (ø)`
...rset-frontend/src/components/ProgressBar/index.tsx	`100.00% <ø> (ø)`
...ontend/src/components/URLShortLinkButton/index.jsx	`100.00% <ø> (ø)`
...src/dashboard/components/dnd/handleScroll/index.ts	`73.33% <ø> (ø)`
...src/dashboard/components/filterscope/treeIcons.jsx	`100.00% <ø> (ø)`
...erset-frontend/src/datasource/DatasourceEditor.jsx	`65.82% <0.00%> (ø)`
... and 143 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db1d598...bf720a0. Read the comment docs.

…o queryScroll

betodealmeida · 2021-03-26T23:08:34Z

/testenv up

github-actions · 2021-03-26T23:08:50Z

@betodealmeida Ephemeral environment creation is currently limited to committers.

junlincc · 2021-03-27T03:33:16Z

/testenv up

@betodealmeida ⬇️

github-actions · 2021-03-27T03:35:30Z

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

junlincc · 2021-03-27T04:05:12Z

Thanks for the PR!

can we add Before vs After screenshot, and manual test plan to the descprtion?
might not be related to the changes in the PR, but please see attached screenshot of the disconnected dropdown

cc @eschutho

AAfghahi · 2021-03-27T13:18:25Z

@junlincc hey! Thank you! I added before shots.

I will happily look into that, thank you for bringing it up.

yousoph · 2021-03-29T06:29:21Z

I think #2 isn't related to this fix, I'm seeing it on master already. Created a separate ticket here: #13842

AAfghahi · 2021-03-29T14:33:01Z

@yousoph and @junlincc played around with it today, I know why it exists, working on fix now.

eschutho · 2021-03-29T23:14:13Z

superset-frontend/src/components/ProgressBar/index.tsx

  .ant-progress-outer {
    ${({ percent }) => !percent && `display: none;`}
  }
  .ant-progress-text {
    font-size: ${({ theme }) => theme.typography.sizes.s}px;
  }
  .ant-progress-bg {
+    position: inherit;


curious, do you know specifically what this value should be? It would be better to be more explicit if you can on these three position values.

no, I don't but I can test it. I think it should probably be absolute or inline-block. That's a very good point, I'll fix it tonight.

@eschutho it was static!

junlincc · 2021-03-29T23:17:55Z

I think #2 isn't related to this fix, I'm seeing it on master already. Created a separate ticket here: #13842

is regression caused by #13694

eschutho

code lgtm. I haven't done any visual testing.

eschutho · 2021-03-30T20:51:38Z

🏷 hold:testing!

eschutho · 2021-04-02T03:07:44Z

superset-frontend/src/SqlLab/components/SouthPane/SouthPane.tsx

@@ -33,7 +33,8 @@ import {
  LOCALSTORAGE_MAX_QUERY_AGE_MS,
 } from '../../constants';

-const TAB_HEIGHT = 64;
+const TAB_HEIGHT = 90;
+const RESULTS_TAB_HEIGHT = TAB_HEIGHT - 10;


nice catch to make this a variable. But are they really related? Or is it just coincidence that they are 10 pixels apart? If they are separate, I would just say RESULTS_TAB_HEIGHT = 80;

@eschutho I was wondering about that. Originally they were the same height, so I thought that they might be related, so that's why I had them work this way, so you only need to change one variable.

Functionally they are not related though

eschutho

Another option is to make the tab heights the same. @Steejay what do you think about that?

AAfghahi · 2021-04-02T03:10:05Z

@eschutho Scratch that, I checked it with the same tab height, and it looks better and is cleaner. TY

eschutho · 2021-04-02T16:30:42Z

Looks good! @Steejay if interested, here's a video.

Screen.Recording.2021-04-02.at.9.25.43.AM.mov

github-actions · 2021-04-02T16:45:09Z

Ephemeral environment shutdown and build artifacts deleted.

(cherry picked from commit 4187d9e)

adjusted tab height

2aa4e03

pull-request-size bot added the size/XS label Mar 26, 2021

Trigger notification

ac24e47

AAfghahi force-pushed the queryScroll branch from 5abd0a5 to ac24e47 Compare March 26, 2021 21:54

AAfghahi added 2 commits March 26, 2021 17:58

adjusted tab height

3d74e4b

Merge branch 'queryScroll' of ssh://github.com/preset-io/superset int…

e57efe7

…o queryScroll

pull-request-size bot added size/S and removed size/XS labels Mar 26, 2021

AAfghahi force-pushed the queryScroll branch from 47c2800 to ebd3cee Compare March 26, 2021 23:10

pull-request-size bot added size/XS and removed size/S labels Mar 26, 2021

fixed progress bar

347e8db

AAfghahi force-pushed the queryScroll branch from ebd3cee to 347e8db Compare March 26, 2021 23:11

pull-request-size bot added size/S and removed size/XS labels Mar 26, 2021

eschutho reviewed Mar 29, 2021

View reviewed changes

eschutho approved these changes Mar 30, 2021

View reviewed changes

junlincc added the hold:testing! On hold for testing label Mar 31, 2021

AAfghahi force-pushed the queryScroll branch from 07d9675 to b539d32 Compare March 31, 2021 22:28

AAfghahi force-pushed the queryScroll branch from b539d32 to adf7f89 Compare March 31, 2021 22:32

changed to static

383d563

AAfghahi force-pushed the queryScroll branch from adf7f89 to 383d563 Compare April 1, 2021 18:48

eschutho reviewed Apr 2, 2021

View reviewed changes

better variable names

bf720a0

AAfghahi force-pushed the queryScroll branch from 7136a95 to bf720a0 Compare April 2, 2021 14:08

eschutho approved these changes Apr 2, 2021

View reviewed changes

hughhhh approved these changes Apr 2, 2021

View reviewed changes

hughhhh merged commit 4187d9e into apache:master Apr 2, 2021

hughhhh deleted the queryScroll branch April 2, 2021 16:44

yousoph mentioned this pull request Apr 5, 2021

[SQL Lab] Can't scroll in SQL Lab Query History (again) #13789

Closed

henryyeh pushed a commit to preset-io/superset that referenced this pull request Apr 6, 2021

fix: adjusted tab height (apache#13822)

3396fe2

(cherry picked from commit 4187d9e)

lyndsiWilliams pushed a commit to preset-io/superset that referenced this pull request Apr 7, 2021

fix: adjusted tab height (apache#13822)

9be9b4f

allanco91 pushed a commit to allanco91/superset that referenced this pull request May 21, 2021

fix: adjusted tab height (apache#13822)

135cfce

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.2.0 labels Mar 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: adjusted tab height #13822

fix: adjusted tab height #13822

AAfghahi commented Mar 26, 2021 •

edited

Loading

codecov bot commented Mar 26, 2021 •

edited

Loading

betodealmeida commented Mar 26, 2021

github-actions bot commented Mar 26, 2021

junlincc commented Mar 27, 2021 •

edited

Loading

github-actions bot commented Mar 27, 2021

junlincc commented Mar 27, 2021 •

edited

Loading

AAfghahi commented Mar 27, 2021

yousoph commented Mar 29, 2021

AAfghahi commented Mar 29, 2021

eschutho Mar 29, 2021 •

edited

Loading

AAfghahi Mar 29, 2021 •

edited

Loading

AAfghahi Mar 30, 2021

eschutho Mar 30, 2021

junlincc commented Mar 29, 2021

eschutho left a comment

eschutho commented Mar 30, 2021

eschutho Apr 2, 2021 •

edited

Loading

AAfghahi Apr 2, 2021 •

edited

Loading

AAfghahi Apr 2, 2021

eschutho left a comment

AAfghahi commented Apr 2, 2021 •

edited

Loading

eschutho commented Apr 2, 2021

github-actions bot commented Apr 2, 2021

fix: adjusted tab height #13822

fix: adjusted tab height #13822

Conversation

AAfghahi commented Mar 26, 2021 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

codecov bot commented Mar 26, 2021 • edited Loading

Codecov Report

betodealmeida commented Mar 26, 2021

github-actions bot commented Mar 26, 2021

junlincc commented Mar 27, 2021 • edited Loading

github-actions bot commented Mar 27, 2021

junlincc commented Mar 27, 2021 • edited Loading

AAfghahi commented Mar 27, 2021

yousoph commented Mar 29, 2021

AAfghahi commented Mar 29, 2021

eschutho Mar 29, 2021 • edited Loading

Choose a reason for hiding this comment

AAfghahi Mar 29, 2021 • edited Loading

Choose a reason for hiding this comment

AAfghahi Mar 30, 2021

Choose a reason for hiding this comment

eschutho Mar 30, 2021

Choose a reason for hiding this comment

junlincc commented Mar 29, 2021

eschutho left a comment

Choose a reason for hiding this comment

eschutho commented Mar 30, 2021

eschutho Apr 2, 2021 • edited Loading

Choose a reason for hiding this comment

AAfghahi Apr 2, 2021 • edited Loading

Choose a reason for hiding this comment

AAfghahi Apr 2, 2021

Choose a reason for hiding this comment

eschutho left a comment

Choose a reason for hiding this comment

AAfghahi commented Apr 2, 2021 • edited Loading

eschutho commented Apr 2, 2021

github-actions bot commented Apr 2, 2021

AAfghahi commented Mar 26, 2021 •

edited

Loading

codecov bot commented Mar 26, 2021 •

edited

Loading

junlincc commented Mar 27, 2021 •

edited

Loading

junlincc commented Mar 27, 2021 •

edited

Loading

eschutho Mar 29, 2021 •

edited

Loading

AAfghahi Mar 29, 2021 •

edited

Loading

eschutho Apr 2, 2021 •

edited

Loading

AAfghahi Apr 2, 2021 •

edited

Loading

AAfghahi commented Apr 2, 2021 •

edited

Loading