@diosmosis opened this Pull Request on March 24th 2022 Member

Description:

Changes:

  • Migrate piwik-segment-generator directive and related model to Vue.

Review

@peterhashair commented on March 28th 2022 Contributor

@diosmosis Overall looks good to me, but I notice when you have a segment that does not exist, it popup the error in the back. The old version doesn't show any info at all. I guess that's an improvement, but should that error banner z-index be higher than the popup?

https://user-images.githubusercontent.com/3783072/160307864-d0a62678-f1db-4568-9e7f-c671b3e7eedf.mp4

@diosmosis commented on March 28th 2022 Member

@peterhashair The notification probably shouldn't have a higher z-index, otherwise it'll look strange. I'm not sure how to fix that UX issue. If it's the same for 4.x-dev perhaps it should be in a new issue. If not, it's some sort of bug.

So I can reproduce it, can you tell me if you have Ecommerce disabled?

@peterhashair commented on March 30th 2022 Contributor

@diosmosis that feature wasn't existing in 4.x-dev, but that is a UX issue, not related to this PR. But there is a JS error on the test see here https://app.travis-ci.com/github/matomo-org/matomo/jobs/564782879

@diosmosis commented on March 30th 2022 Member

@peterhashair those are the tests for the tracker JavaScript, this PR shouldn't relate to those.

@peterhashair commented on March 31st 2022 Contributor

@peterhashair those are the tests for the tracker JavaScript, this PR shouldn't relate to those.

@diosmosis sorry, wasn't looking through details.

This Pull Request was closed on March 31st 2022
Powered by GitHub Issue Mirror