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

Jump to bottom

Hide date selector when viewing live visits (not as widget) #14560

Merged
merged 2 commits into from Jul 5, 2019
Merged

Hide date selector when viewing live visits (not as widget) #14560

merged 2 commits into from Jul 5, 2019

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Jun 23, 2019

fixes #14517

@sgiehl sgiehl added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Jun 23, 2019
@sgiehl sgiehl added this to the 3.11.0 milestone Jun 23, 2019
@diosmosis
Copy link
Member

Works for me. Only thing that worries me is if some error occurs and _destroy() isn't called. This would result in the period selector being hidden, which could be an annoying bug to track down.

@sgiehl
Copy link
Member Author

sgiehl commented Jun 25, 2019

@diosmosis would it be better like it's done in the real time map:

matomo/plugins/UserCountryMap/javascripts/realtime-map.js

Lines 63 to 71 in 8f77ece

var $rootScope = piwikHelper.getAngularDependency('$rootScope');
$rootScope.$on('piwikPageChange', function () {
var href = location.href;
var clickedMenuIsNotMap = !href || (href.indexOf('module=UserCountryMap&action=realtimeWorldMap') == -1);
if (clickedMenuIsNotMap) {
$('#periodString').show();
initTopControls();
}
});

@diosmosis
Copy link
Member

Huh, I would expect that might cause problems w/ this change actually, since I think the event is never removed from $rootScope?

@sgiehl
Copy link
Member Author

sgiehl commented Jul 1, 2019

I actually didn't have problems with that while testing. Guess the piwikPageChange event is triggered before the widgets are created or destroyed.
I'll try to implement something else, that simplifies both implementations....

@sgiehl
Copy link
Member Author

sgiehl commented Jul 1, 2019

@diosmosis tried to improve & simplify the code. Could you have another look?

diosmosis
diosmosis approved these changes Jul 1, 2019
View reviewed changes
Copy link
Member

@diosmosis diosmosis left a comment

Choose a reason for hiding this comment

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

haven't tested, but this looks great

@tsteur tsteur merged commit 1d0cc72 into 3.x-dev Jul 5, 2019
@tsteur tsteur deleted the hidedateselectorforlive branch July 5, 2019 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@diosmosis diosmosis diosmosis approved these changes

Assignees
No one assigned
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Milestone
3.11.0
Development

Successfully merging this pull request may close these issues.

In Visitors > Real-time report, hide the calendar
3 participants
@sgiehl @diosmosis @tsteur