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: Issue #12004 timegrain not visibile #12258

Merged
merged 4 commits into from
Jan 6, 2021
Merged

fix: Issue #12004 timegrain not visibile #12258

merged 4 commits into from
Jan 6, 2021

Conversation

geido
Copy link
Member

@geido geido commented Jan 4, 2021

SUMMARY

Fixes an issue that makes the SelectControl options hidden when overflowing.

TEST PLAN

  1. Go to a Dashboard
  2. Inside a Filter box open the options of any single-choice dropdown (multi-options dropdowns already worked properly)
  3. Make sure the options are visible even when overflowing the filter box container

ADDITIONAL INFORMATION

@geido
Copy link
Member Author

geido commented Jan 4, 2021

@rusackas @junlincc

@codecov-io
Copy link

codecov-io commented Jan 4, 2021

Codecov Report

Merging #12258 (dd55c51) into master (164db3e) will decrease coverage by 2.35%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12258      +/-   ##
==========================================
- Coverage   66.59%   64.23%   -2.36%     
==========================================
  Files         996      485     -511     
  Lines       49194    29806   -19388     
  Branches     4994        0    -4994     
==========================================
- Hits        32759    19146   -13613     
+ Misses      16298    10660    -5638     
+ Partials      137        0     -137     
Flag Coverage Δ
cypress ?
javascript ?
python 64.23% <ø> (ø)

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

Impacted Files Coverage Δ
...hboard/components/filterscope/FilterScopeModal.tsx
...et-frontend/src/dashboard/components/SaveModal.tsx
superset-frontend/src/dashboard/reducers/index.js
superset-frontend/src/showSavedQuery/index.jsx
...rontend/src/views/CRUD/dashboard/DashboardList.tsx
superset-frontend/src/views/CRUD/data/hooks.ts
superset-frontend/src/components/TableSelector.tsx
superset-frontend/src/utils/errorMessages.ts
...uperset-frontend/src/components/ExpandableList.tsx
...ws/CRUD/data/savedquery/SavedQueryPreviewModal.tsx
... and 500 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 164db3e...dd55c51. Read the comment docs.

Copy link
Member

@junlincc junlincc left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix Diego!
manually tested. when i resize the filter box with Timegrain to really small, the dropdown got disconnected as I scroll.

Screen.Recording.2021-01-04.at.3.33.48.PM.mov

couple suggestions for futre:

  1. add a video to all the PRs that have UI change or fix
  2. test the feature throughout, resize the window or component from different directions

@geido
Copy link
Member Author

geido commented Jan 5, 2021

@junlincc this is a problem with react-select and this issue was affecting the multi-choice dropdowns already. The issue is known and it is still open. You can read the details here JedWatson/react-select#4088. I will attempt a few possible fixes but there is a high chance that there is no optimal solution for this right now.

@junlincc
Copy link
Member

junlincc commented Jan 5, 2021

@junlincc this is a problem with react-select and this issue was affecting the multi-choice dropdowns already. The issue is known and it is still open. You can read the details here JedWatson/react-select#4088. I will attempt a few possible fixes but there is a high chance that there is no optimal solution for this right now.

got it! thanks for letting me know. if the code looks good, alright to merge :) thanks for the fix!

Copy link
Contributor

@nikolagigic nikolagigic left a comment

Choose a reason for hiding this comment

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

LGTM

@rusackas rusackas merged commit 147605c into apache:master Jan 6, 2021
@junlincc junlincc added the dashboard:filterbox Related to the Dashboard filterbox viz type label Jan 8, 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
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 dashboard:filterbox Related to the Dashboard filterbox viz type size/XS 🚢 1.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants