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: Time filter position and click in Horizontal FilterBar #22338

Merged
merged 4 commits into from
Dec 8, 2022

Conversation

geido
Copy link
Member

@geido geido commented Dec 6, 2022

SUMMARY

Fixes positioning of the Time filter popover as well as a bug that would close the popover when clicking on the filter from within the Horizontal FilterBar dropdown container.

BEFORE

master.mp4

AFTER

1.-.Sample.dashboard.mp4

TESTING INSTRUCTIONS

See repro steps in the videos

ADDITIONAL INFORMATION

  • [] Has associated issue:
  • Required feature flags: HORIZONTAL_FILTER_BAR
  • 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

@codecov
Copy link

codecov bot commented Dec 6, 2022

Codecov Report

Merging #22338 (2ac5f5a) into master (e1ffdb9) will decrease coverage by 0.01%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##           master   #22338      +/-   ##
==========================================
- Coverage   66.85%   66.83%   -0.02%     
==========================================
  Files        1847     1847              
  Lines       70561    70564       +3     
  Branches     7737     7741       +4     
==========================================
- Hits        47174    47163      -11     
- Misses      21380    21394      +14     
  Partials     2007     2007              
Flag Coverage Δ
hive 52.53% <ø> (ø)
javascript 53.80% <80.00%> (+<0.01%) ⬆️
mysql ?
postgres 78.02% <ø> (ø)
presto 52.42% <ø> (ø)
python 81.19% <ø> (-0.05%) ⬇️
sqlite 76.48% <ø> (ø)
unit 50.92% <ø> (ø)

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

Impacted Files Coverage Δ
...ore/components/controls/DateFilterControl/types.ts 100.00% <ø> (ø)
...d/src/filters/components/Time/TimeFilterPlugin.tsx 0.00% <0.00%> (ø)
...tend/src/filters/components/Time/transformProps.ts 0.00% <ø> (ø)
...rset-frontend/src/filters/components/Time/types.ts 100.00% <ø> (ø)
...nts/controls/DateFilterControl/DateFilterLabel.tsx 55.20% <50.00%> (-0.12%) ⬇️
...ponents/controls/ControlPopover/ControlPopover.tsx 92.72% <100.00%> (+3.63%) ⬆️
superset/common/utils/dataframe_utils.py 90.47% <0.00%> (-4.77%) ⬇️
superset/db_engine_specs/mysql.py 94.04% <0.00%> (-4.77%) ⬇️
superset/models/core.py 89.81% <0.00%> (-0.70%) ⬇️
superset/views/core.py 74.98% <0.00%> (-0.46%) ⬇️

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

@geido
Copy link
Member Author

geido commented Dec 6, 2022

Let’s confirm that this isn’t breaking anything new, especially from the original PR that introduced the positioning fixes CC @jinghua-qa

@geido geido requested a review from jinghua-qa December 6, 2022 15:14
@michael-s-molina
Copy link
Member

Tagging @diegomedina248 too.

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. Can we get @jinghua-qa and @diegomedina248 approvals too?

Copy link
Contributor

@diegomedina248 diegomedina248 left a comment

Choose a reason for hiding this comment

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

LGTM

@geido
Copy link
Member Author

geido commented Dec 6, 2022

/testenv up

@github-actions
Copy link
Contributor

github-actions bot commented Dec 6, 2022

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

@jinghua-qa
Copy link
Member

/testenv up FEATURE_HORIZONTAL_FILTER_BAR

@github-actions
Copy link
Contributor

github-actions bot commented Dec 8, 2022

@jinghua-qa Ephemeral environment spinning up at http://52.41.118.83:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

Copy link
Member

@jinghua-qa jinghua-qa left a comment

Choose a reason for hiding this comment

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

LGTM

@kgabryje kgabryje merged commit f64423a into master Dec 8, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Dec 8, 2022

Ephemeral environment shutdown and build artifacts deleted.

jinghua-qa pushed a commit to preset-io/superset that referenced this pull request Dec 10, 2022
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 and removed 🚢 2.1.3 labels Mar 13, 2024
@mistercrunch mistercrunch deleted the fix/dropdown-container-click-out branch March 26, 2024 16:11
# 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 preset:2022.49 size/M 🚢 2.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants