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: Explore popovers issues (#11428) #35

Closed
wants to merge 1 commit into from

Conversation

mistercrunch
Copy link
Member

@mistercrunch mistercrunch commented Oct 31, 2020

This cherry is essentially/simpy apache#11428 with a single/simple merge conflict fixed in Cypress tests, it's on top 5579772

Simply git cherry-pick -x 45e767a7dfbc2b1d3c5078324f0ffc97dc571560 to pick on top of the current release

* Fix spaces and comas not working in filter popover

* Fix popup not opening automatically

* Add e2e test

* Remove only from test

* Remove redundant test, add checking label content

* Add comments to e2e test

* Fix unit test

* Use destructuring

* Always open popup for functions and saved metrics, too

* Fix popover for adhoc metrics too

* Small refactor to consistency

* Refactor for consistency

* Remove redundant functions

* Test fix

Co-authored-by: Jesse Yang <jesse.yang@airbnb.com>
(cherry picked from commit b2636f0)
@codecov-io
Copy link

codecov-io commented Oct 31, 2020

Codecov Report

Merging #35 into preset/release-43-branch will decrease coverage by 4.19%.
The diff coverage is 41.93%.

Impacted file tree graph

@@                     Coverage Diff                      @@
##           preset/release-43-branch      #35      +/-   ##
============================================================
- Coverage                     65.79%   61.60%   -4.20%     
============================================================
  Files                           838      838              
  Lines                         39874    39867       -7     
  Branches                       3655     3654       -1     
============================================================
- Hits                          26237    24559    -1678     
- Misses                        13536    15127    +1591     
- Partials                        101      181      +80     
Flag Coverage Δ
cypress ?
javascript 62.62% <41.93%> (-0.02%) ⬇️
python 60.99% <ø> (ø)

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

Impacted Files Coverage Δ
...explore/components/controls/AdhocFilterControl.jsx 60.19% <ø> (-17.48%) ⬇️
...ntend/src/explore/components/AdhocFilterOption.jsx 57.14% <35.29%> (-23.51%) ⬇️
...ntend/src/explore/components/AdhocMetricOption.jsx 65.62% <46.15%> (-25.00%) ⬇️
...uperset-frontend/src/common/components/Popover.tsx 100.00% <100.00%> (ø)
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/explore/index.jsx 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/dashboard/index.jsx 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/setup/setupColors.js 0.00% <0.00%> (-100.00%) ⬇️
... and 170 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 5579772...45e767a. Read the comment docs.

@mistercrunch mistercrunch changed the base branch from preset/release-43-branch to release-43 October 31, 2020 16:50
preset-machine pushed a commit that referenced this pull request Nov 30, 2021
* Fix issue with large timestamp arrays

Math.max and Math.min produce "Maximum call stack size exceeded" errors when used with large arrays (see: https://stackoverflow.com/questions/18308700/chrome-how-to-solve-maximum-call-stack-size-exceeded-errors-on-math-max-apply)

* fix issue when viewport.zoom < 0

* Update packages/superset-ui-legacy-preset-chart-deckgl/src/utils/time.js

Sure. Style changed

Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com>

Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants