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

Couple more period selector related improvements #12326

Merged
merged 5 commits into from Jul 26, 2018

Conversation

diosmosis
Copy link
Member

@diosmosis diosmosis commented Dec 4, 2017

Changes:

  • Changing what piwik.currentDateString is meant to contain. Before it held the date query parameter value, unless the period was a range, in which case it held the last date in the date range (so w/ 2011-01-01,2011-01-23, the date would be 2011-01-23). Now it will contain the two dates for ranges. Note: not sure if this will have any effect on other plugins, but I think core plugins are safe. I noticed it's only used by the managecustomalerts controller.

  • Supply the site's timezone client side for calculating now/today/previousN/lastN. Note: this works for now, but I'm not sure if this is the best way to supply site info to the client.

  • Remove blip when hovering over dates in the date-picker directive.

  • For sanity, handle yesterdaySameTime client side (handled same as yesterday, since the frontend doesn't care about the time of day for periods).

@mattab mattab added this to the 3.4.0 milestone Jan 10, 2018
@diosmosis diosmosis added the Needs Review PRs that need a code review label Jun 14, 2018
@sgiehl sgiehl force-pushed the period-selector-improvements branch from 6edb020 to 6e08e51 Compare July 16, 2018 13:06
Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

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

Code changes look good. We should maybe change the timezone offset calculation in scheduled reports plugin to reuse the stuff added here...

@diosmosis
Copy link
Member Author

@sgiehl will look into it

@diosmosis
Copy link
Member Author

@sgiehl simplified the code a bit based on your suggestion to look in ScheduledReports, I think it merits another quick review

@sgiehl
Copy link
Member

sgiehl commented Jul 26, 2018

looks good 👍

@diosmosis diosmosis merged commit 70d1b44 into 3.x-dev Jul 26, 2018
@diosmosis diosmosis deleted the period-selector-improvements branch July 26, 2018 21:27
InfinityVoid pushed a commit to InfinityVoid/matomo that referenced this pull request Oct 11, 2018
* Changing semantics of piwik.currentDateString, making it always the value of date query param instead of that or the first date str in a range period.

* Use site timezone in getting todays date client side.

* For slightly smoother rendering, override jquery UI event handler in date-picker directive.

* Just to be comprehensive, handle yesterdaySameTime client side as well.

* Simplify additions based on existing code & reuse in scheduled reports.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants