Skip to content

fix: Blank space in Change dataset modal without warning message#16324

Merged
geido merged 2 commits into
apache:masterfrom
geido:fix/issue_16291_space_dataset_modal
Aug 19, 2021
Merged

fix: Blank space in Change dataset modal without warning message#16324
geido merged 2 commits into
apache:masterfrom
geido:fix/issue_16291_space_dataset_modal

Conversation

@geido
Copy link
Copy Markdown
Member

@geido geido commented Aug 18, 2021

SUMMARY

This PR should definitely fix a problem with a blank space appearing at the bottom of the Change dataset modal when closing the top warning whether pagination is on or off.

BEFORE

change_dataset_modal_before.mp4

AFTER WITHOUT PAGINATION

change_dataset_modal_no_pagination_after.mp4

AFTER WITH PAGINATION

change_dataset_modal_with_pagination_after.mp4

TESTING INSTRUCTIONS

  1. Open Explore
  2. Open the Change Dataset modal
  3. Close the top warning
  4. Make sure no excessive blank space appears at the bottom

ADDITIONAL INFORMATION

  • Has associated issue: Fixes explore: Blank space in Change dataset modal when warning is closed #16291
  • Required feature flags:
  • 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

@geido
Copy link
Copy Markdown
Member Author

geido commented Aug 18, 2021

As we were discussing with @michael-s-molina it may make sense to make the warning not closable. The warning will appear again when re-opening the modal anyway. CC @junlincc

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 18, 2021

Codecov Report

Merging #16324 (6a59710) into master (2c595b0) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #16324   +/-   ##
=======================================
  Coverage   76.63%   76.63%           
=======================================
  Files        1000     1000           
  Lines       53426    53426           
  Branches     6819     6818    -1     
=======================================
  Hits        40945    40945           
  Misses      12244    12244           
  Partials      237      237           
Flag Coverage Δ
javascript 70.77% <100.00%> (ø)

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

Impacted Files Coverage Δ
...et-frontend/src/components/TableView/TableView.tsx 94.52% <ø> (-0.08%) ⬇️
...-frontend/src/datasource/ChangeDatasourceModal.tsx 87.50% <100.00%> (+0.14%) ⬆️

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 2c595b0...6a59710. Read the comment docs.

@geido
Copy link
Copy Markdown
Member Author

geido commented Aug 18, 2021

/testenv up

@github-actions
Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Member

@pkdotson pkdotson left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Copy Markdown
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.

product signoff ✅❤️

@geido geido merged commit c5c2861 into apache:master Aug 19, 2021
@github-actions
Copy link
Copy Markdown
Contributor

Ephemeral environment shutdown and build artifacts deleted.

amitmiran137 pushed a commit to nielsen-oss/superset that referenced this pull request Aug 24, 2021
* master: (582 commits)
  feat: config to customize bootstrap data overrides (apache#16386)
  fix: regex for multi-region IPs (apache#16410)
  chore: Changes the DatabaseSelector to use the new Select component (apache#16334)
  chore: Displays the dataset description in a tooltip in the datasets list (apache#16392)
  fix(pylint): Fix master (apache#16405)
  chore(pylint): Enable useless-suppression check (apache#16388)
  feat: Add extraVolumes and extraVolumeMounts to all main containers (apache#16361)
  initial commit (apache#16366)
  fix: big number default date format (apache#16383)
  initial commit (apache#16380)
  fix: import dashboard w/o metadata (apache#16360)
  test: Functional RTL for email report modal II (apache#16148)
  fix: update table ID in query context on chart import (apache#16374)
  docs: document FLASK_APP_MUTATOR (apache#16286)
  feat: Add new dev commands to Makefile (apache#16327)
  fix: call external metadata endpoint with correct rison object (apache#16369)
  fix: Fix parsing onSaving reports toast when user hasn't saved chart (apache#16330)
  fix: columns/index rebuild (apache#16355)
  chore(viz): bump deckgl plugin to 0.4.11 (apache#16353)
  fix: Blank space in Change dataset modal without warning message (apache#16324)
  ...

# Conflicts:
#	superset/app.py
#	superset/models/dashboard.py
#	tests/integration_tests/charts/filter_sets/__init__.py
#	tests/integration_tests/charts/filter_sets/conftest.py
#	tests/integration_tests/charts/filter_sets/consts.py
#	tests/integration_tests/charts/filter_sets/create_api_tests.py
#	tests/integration_tests/charts/filter_sets/delete_api_tests.py
#	tests/integration_tests/charts/filter_sets/get_api_tests.py
#	tests/integration_tests/charts/filter_sets/update_api_tests.py
#	tests/integration_tests/charts/filter_sets/utils.py
#	tests/integration_tests/superset_test_config.py
opus-42 pushed a commit to opus-42/incubator-superset that referenced this pull request Nov 14, 2021
QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 28, 2021
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.4.0 First shipped in 1.4.0 labels Mar 13, 2024
qfcwell pushed a commit to qfcwell/superset that referenced this pull request May 12, 2026
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 size/S 🚢 1.4.0 First shipped in 1.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

explore: Blank space in Change dataset modal when warning is closed

4 participants