fix: Jumpiness of datasource modal

[geido]

SUMMARY

Partially fixes #12503

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

BEFORE:

BEFORE-jumpiness-Num.Births.Trend.mp4

AFTER:

AFTER-jumpinessNum.Births.Trend.mp4

TEST PLAN

1. Open the datasource modal in Explore
2. Make sure the modal has a fixed height and jumpiness is removed

ADDITIONAL INFORMATION

• Has associated issue: [Explore]modal jumpiness is back, added scroll bar outside the modal #12503
• Changes UI
• Requires DB Migration.
• Confirm DB Migration upgrade and downgrade tested.
• Introduces new feature or API
• Removes existing feature or API

[geido]

@junlincc

[adam-stasiak]

I see that modal for changing dataset is now very small and has additional line paris_iris_mapping and multiformat_time_series:

Nagranie.z.ekranu.2021-01-13.o.22.26.03.mov

[codecov-io]

Codecov Report

Merging #12502 (3dae9f5) into master (57e37ed) will decrease coverage by 4.37%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #12502      +/-   ##
==========================================
- Coverage   66.72%   62.34%   -4.38%     
==========================================
  Files        1014     1014              
  Lines       49612    49594      -18     
  Branches     4839     4840       +1     
==========================================
- Hits        33102    30919    -2183     
- Misses      16387    18477    +2090     
- Partials      123      198      +75     

Flag	Coverage Δ
cypress	?
javascript	60.73% <100.00%> (+<0.01%)	⬆️
python	63.39% <ø> (-0.52%)	⬇️

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

Impacted Files	Coverage Δ
...-frontend/src/datasource/ChangeDatasourceModal.tsx	85.18% <100.00%> (+1.23%)	⬆️
superset-frontend/src/SqlLab/App.jsx	0.00% <0.00%> (-100.00%)	⬇️
superset-frontend/src/explore/App.jsx	0.00% <0.00%> (-100.00%)	⬇️
superset-frontend/src/dashboard/App.jsx	0.00% <0.00%> (-100.00%)	⬇️
superset-frontend/src/chart/ChartContainer.jsx	0.00% <0.00%> (-100.00%)	⬇️
superset-frontend/src/explore/reducers/index.js	0.00% <0.00%> (-100.00%)	⬇️
...et-frontend/src/dashboard/containers/Dashboard.jsx	0.00% <0.00%> (-100.00%)	⬇️
...et-frontend/src/filters/components/Select/types.ts	0.00% <0.00%> (-100.00%)	⬇️
...t-frontend/src/dashboard/containers/SliceAdder.jsx	0.00% <0.00%> (-100.00%)	⬇️
...t-frontend/src/explore/reducers/getInitialState.js	0.00% <0.00%> (-100.00%)	⬇️
... and 195 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 57e37ed...3dae9f5. Read the comment docs.

[geido]

@adam-stasiak I will increase the height a bit. The other issue you are mentioning seems unrelated but I'll have a look. Thanks for your feedback!

[adam-stasiak]

Thank you - I compared other issue place on master and did not observe this. I did not spot any other issues.

[geido]

Hello @adam-stasiak. What I have found out is that the footer of the Modal was showing even though it was not supposed to. The problem wasn't visible on master as the footer went all the way down, but it becomes visible by reducing the height. I have fixed that issue and also increased the height as you suggested. Please have a look

Path.mp4

[ktmud]

Is this a stop-gap fix? Because the dataset modal isn't implemented as described in #12503 .

Only the table body should be scrollable. We should replace the dataset table with the same implementation as in the table viz or Data preview on the Explore page.

[geido]

Hello @ktmud. I believe the issue you are referring to is this #12488 and it's in progress in a separate PR.

[ktmud]

#12503 also described how the scroll area should look like.

[junlincc]

Thanks for updating the title and description. after removing the code that attempts to fix the dropdown issue globally, this PR should be ready for testing again - jumpiness in Change dataset modal. @geido

[geido]

I think I am now seeing the issue. I am not confident anymore to apply the fix globally to the modals as we might end up in scenarios like the one depicted here #11770 (comment). I am only including the fix to the jumpiness in this PR as of now.

For the "Save" modal issue specifically, this #12522 should be preferred even though it is not a global fix.

As for the sticky components described in #12488 and #12503 I'll be providing them in a separate PR asap.

@junlincc @ktmud

[ktmud]

Thank you for the diligent work, @geido ! Yes, I agree it's better to address the jumpiness and scrollable areas in separate PRs.

[junlincc]

oh well, between jumpiness and the regression I introduced ... i chose the jumpiness 🤦🏾‍♀️
let's wait for the other PR that address the sticky issue first, then merge both together.
@geido going forward, could you test your PR by scrolling, resizing the window. messing around in nearby area to make sure everything looks ok?

[geido]

Let's open a PR addressing the modal issues as discussed. I am closing this one.