Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: Datetime popover issues #11621

Merged
merged 1 commit into from
Nov 9, 2020
Merged

Conversation

kgabryje
Copy link
Member

@kgabryje kgabryje commented Nov 9, 2020

SUMMARY

Fixes issues described in #11609 and #11601.
I fixed the overflowing calendar issue by overriding tabs overflow styles in the popover. I don't think it's a perfect solution, because if the content of the tab changes to something that would actually require overflow, we'll need to do some workarounds.
I tried to render the calendar in a Portal (using renderView prop), outside of nodes with overflow: hidden, but react-datetime doesn't work well with portals - the calendar view relies on its parent's css class, so we can't mount it in a different place.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before: see linked issues
After:
ezgif com-gif-maker (2)

TEST PLAN

ADDITIONAL INFORMATION

CC: @junlincc, @graceguo-supercat, @mistercrunch

@kgabryje
Copy link
Member Author

kgabryje commented Nov 9, 2020

@rusackas Can you take a look? What do you think about using overflow: visible?

@codecov-io
Copy link

codecov-io commented Nov 9, 2020

Codecov Report

Merging #11621 (b9f719a) into master (92a9acd) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #11621   +/-   ##
=======================================
  Coverage   62.26%   62.26%           
=======================================
  Files         873      873           
  Lines       42238    42238           
  Branches     3959     3959           
=======================================
  Hits        26301    26301           
  Misses      15757    15757           
  Partials      180      180           
Flag Coverage Δ
javascript 62.84% <ø> (ø)
python 61.91% <ø> (ø)

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

Impacted Files Coverage Δ
.../explore/components/controls/DateFilterControl.jsx 55.49% <ø> (ø)

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 92a9acd...b9f719a. Read the comment docs.

@junlincc junlincc requested a review from rusackas November 9, 2020 16:49
@mistercrunch mistercrunch added the rush! Requires immediate attention label Nov 9, 2020
Copy link
Member

@rusackas rusackas left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the quick fix.

@rusackas rusackas merged commit 0c6aeef into apache:master Nov 9, 2020
henryyeh pushed a commit to preset-io/superset that referenced this pull request Nov 9, 2020
auxten pushed a commit to auxten/incubator-superset that referenced this pull request Nov 20, 2020
@junlincc junlincc removed the rush! Requires immediate attention label Mar 18, 2021
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.0.0 labels Mar 12, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/XS 🚢 1.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants