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 new transitions report #13475
Add new transitions report #13475
Conversation
@mattab UI needs approval |
self.isEnabled = false; | ||
self.actionName = ''; | ||
self.actionNameOptions.push({key: '', value: translate('CoreHome_ThereIsNoDataForThisReport')}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not super important, but I think this method could be split up for better readability, if desired.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will do 👍
I didn't manage to center it but I actually think it's fine like this anyway. Feel free to give it a try. It's so hard to do much there I think cause the transitions JS is quite complex and built for popup etc.
had actually no idea it was clickable... i think the JS is not able to render 2 transition reports on the same page or it's trying to alter something.
should be fixed also applied the other feedback @diosmosis |
LGTM just need UX approval from @mattab & UI tests |
fyi: merged 3.x-dev into this PR. Fixed system test & ui tests. Added UI tests (need to update again once the build has finished). The UI test show something funny for transitions page titles but checked and this seems to be actually a general issue of transition, not related to the integration etc. UI should be fine for now, don't think we need to wait for it to merge re @mattab |
Tests should be fixed now. |
Was going to merge, but the transitions widget looks a bit off: Guess the page title shouldn't display in the dashboard. I think something in the code is also causing the UI test on travis to have issues (works fine for me locally): https://builds-artifacts.matomo.org/matomo-org/matomo/12865/31592/UIIntegrationTest_dashboard3.png Think it might have something to do w/ the width of an element. |
@diosmosis I think I fixed the remaining issues this morning |
Looks really good overall, it's nice to know more users will discover this feature.
|
|
As you can maybe see in the first commit I tried to first make it work as a report with a visualization and including related report etc but couldn't get it to work. There were various challenges. So made it a widget instead. Was still quite tricky to get it to work and is a bit messy since it was all hard adjusted to be a popover.
Looks like this:
No data screen:
Menu:
There is no search in the label field for now. It's only our regular select. Should do for now I'd say and at some point need to add support for autocomplete or so.
I'll try to add some UI tests if that's looking good @mattab
fix #12865