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

Add play slider to screengrid #4647

Merged
merged 7 commits into from
Apr 9, 2018

Conversation

betodealmeida
Copy link
Member

Note: this is based on #4623.

screengrid

@codecov-io
Copy link

codecov-io commented Mar 19, 2018

Codecov Report

Merging #4647 into master will decrease coverage by 0.03%.
The diff coverage is 46.66%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #4647      +/-   ##
=========================================
- Coverage   72.63%   72.6%   -0.04%     
=========================================
  Files         205     206       +1     
  Lines       15403   15418      +15     
  Branches     1183    1184       +1     
=========================================
+ Hits        11188   11194       +6     
- Misses       4212    4221       +9     
  Partials        3       3
Impacted Files Coverage Δ
...set/assets/javascripts/explore/stores/controls.jsx 39.25% <ø> (ø) ⬆️
superset/viz.py 78.94% <100%> (ø) ⬆️
superset/assets/javascripts/modules/time.js 20% <20%> (ø)

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 41c158e...979f7c7. Read the comment docs.


const pattern = new RegExp('^(\\d+)?\\s*(.*)$');

export const parseTimeGrain = function (timeGrain) {
Copy link
Member

Choose a reason for hiding this comment

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

Wondering if we can have a single authority on time grain parsing between the frontend & backend. Backend has utils.parse_human_datetime leveraging the parsedatetime lib. Just worried there's divergence into what the backend and frontend understand.

The backend could return its understanding of granularity to be available to the frontend in the payload. This may require a bit more thinking.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that would definitely be better. We can pass an ISO 8601 duration as a string. Let me take a look at it.

@betodealmeida
Copy link
Member Author

@mistercrunch, I rebased this and now it uses the ISO duration parsing.

@mistercrunch mistercrunch merged commit 47c085f into apache:master Apr 9, 2018
@mistercrunch mistercrunch deleted the DPTOOLS-389_play_slider_screengrid branch April 9, 2018 21:02
michellethomas pushed a commit to michellethomas/panoramix that referenced this pull request May 24, 2018
* Improved granularity parsing

* Add unit tests

* Explicit cast to int

* Screengrid play slider

* Clean code

* Refactor common code
timifasubaa pushed a commit to timifasubaa/incubator-superset that referenced this pull request May 31, 2018
* Improved granularity parsing

* Add unit tests

* Explicit cast to int

* Screengrid play slider

* Clean code

* Refactor common code
wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018
* Improved granularity parsing

* Add unit tests

* Explicit cast to int

* Screengrid play slider

* Clean code

* Refactor common code
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.25.0 labels Feb 27, 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 🚢 0.25.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants