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

feat: default timepicker to last week when dataset is changed #12609

Merged
merged 6 commits into from
Jan 26, 2021

Conversation

pkdotson
Copy link
Member

@pkdotson pkdotson commented Jan 19, 2021

SUMMARY

This pr defaults the time picker in chart control panel to last week when user changes dataset. This is the first step in setting up the controls for metrics to default to empty states when user changes datasets.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen.Recording.2021-01-22.at.3.36.21.PM.mov

TEST PLAN

Currently manually testing and updating related tests as necessary.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • [ x] Introduces new feature or API
  • Removes existing feature or API

@pkdotson pkdotson marked this pull request as draft January 19, 2021 23:19
@codecov-io
Copy link

codecov-io commented Jan 19, 2021

Codecov Report

Merging #12609 (e899383) into master (8f83afc) will increase coverage by 3.89%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12609      +/-   ##
==========================================
+ Coverage   62.77%   66.66%   +3.89%     
==========================================
  Files        1015     1022       +7     
  Lines       49537    49999     +462     
  Branches     5079     4894     -185     
==========================================
+ Hits        31096    33333    +2237     
+ Misses      18232    16541    -1691     
+ Partials      209      125      -84     
Flag Coverage Δ
cypress 50.91% <66.66%> (?)
javascript 60.85% <0.00%> (+0.08%) ⬆️
python 63.72% <ø> (-0.37%) ⬇️

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

Impacted Files Coverage Δ
.../src/explore/components/ControlPanelsContainer.jsx 91.02% <ø> (+11.27%) ⬆️
...s/controls/DateFilterControl/DateFilterControl.tsx 83.62% <66.66%> (+64.52%) ⬆️
superset/db_engines/hive.py 0.00% <0.00%> (-85.72%) ⬇️
superset/examples/world_bank.py 30.98% <0.00%> (-69.02%) ⬇️
superset/examples/helpers.py 85.36% <0.00%> (-12.20%) ⬇️
...frontend/src/dashboard/containers/FiltersBadge.tsx 89.28% <0.00%> (-10.72%) ⬇️
superset/db_engine_specs/hive.py 73.84% <0.00%> (-10.00%) ⬇️
superset/views/datasource.py 89.39% <0.00%> (-5.61%) ⬇️
...d/src/views/CRUD/csstemplates/CssTemplateModal.tsx 67.61% <0.00%> (-4.83%) ⬇️
...rontend/src/explore/components/DatasourcePanel.tsx 82.60% <0.00%> (-3.44%) ⬇️
... and 288 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 8f83afc...e899383. Read the comment docs.

@junlincc junlincc added the explore:control Related to the controls panel of Explore label Jan 20, 2021
@pull-request-size pull-request-size bot added size/S and removed size/L labels Jan 22, 2021
@pkdotson pkdotson marked this pull request as ready for review January 22, 2021 18:18
@junlincc junlincc requested review from zhaoyongjie and ktmud January 23, 2021 06:14
@junlincc
Copy link
Member

@pkdotson
thanks for the PR!

  1. when user change a dataset, control panel reset time range to Last week automatically on pill ✅
  2. but when you click the pill and open popover, notice it's still the time range setting, in this case, last year -> not matching the pill
Screen.Recording.2021-01-22.at.10.56.39.PM.mov

@junlincc junlincc self-requested a review January 23, 2021 07:02
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.

changes LGTM, thank you Phillip!
et's move forward with rest of the controls - changing dataset clear all previous input.
ezgif-3-a70aae2ebf9c

Copy link
Member

@zhaoyongjie zhaoyongjie left a comment

Choose a reason for hiding this comment

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

LGTM, just a few nits.

Copy link
Member

@zhaoyongjie zhaoyongjie left a comment

Choose a reason for hiding this comment

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

LGTM, just a few nits.

Comment on lines +227 to +228
setTimeRangeValue('Last week');
setFrame(guessFrame('Last week'));
Copy link
Member

Choose a reason for hiding this comment

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

These two lines can be removed. Because when the value changed(onChange('last week')) then the component can reload.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry. Ignore me. Shouldn't be removed. merged.

@zhaoyongjie zhaoyongjie merged commit a5df4c6 into apache:master Jan 26, 2021
amitmiran137 pushed a commit to nielsen-oss/superset that referenced this pull request Jan 26, 2021
* master: (52 commits)
  docs: Updates to Superset Site for 1.0 (apache#12626)
  test(native-filters): scoping tree in native filters modal (apache#12655)
  Fix tests errors and warnings - iteration 3 (apache#12212) (apache#12219)
  Fix tests errors and warnings - iteration 5 (apache#12212) (apache#12224)
  Fix tests errors and warnings - iteration 6 (apache#12212) (apache#12227)
  feat(native-filters): apply scoping of native filters to dashboard (apache#12716)
  Fix tests errors and warnings - iteration 4 (apache#12212) (apache#12223)
  Fix tests errors and warnings - iteration 7 (apache#12212) (apache#12245)
  fix: missing select menu background (apache#12759)
  fix(explore): incorrect missing datasource condition (apache#12758)
  feat: default timepicker to last week when dataset is changed (apache#12609)
  feat(explore): allow opening charts with missing dataset (apache#12705)
  chore: upgrade Cypress to 6.2.1 (apache#12605)
  refactor(explore): Enhance Dataset and Control panel Collapse components (apache#12218)
  feat: Adding option to set_database_uri CLI command (apache#12740)
  docs: Fixed typo on line 348 (apache#12739)
  Fix tests errors and warnings - iteration 2 (apache#12212) (apache#12214)
  docs: Remove gatsby-plugin-offline (apache#12693)
  test: oracle engine spec (apache#12615)
  test: hive db engine spec (apache#12520)
  ...
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.2.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 explore:control Related to the controls panel of Explore preset-io size/S 🚢 1.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants