add feedback question, remove other related. #18262

peterhashair · 2021-11-03T01:58:43Z

Description:

Fixes:#18251 #18250 #18232

merge @andyjdavis #18277 remove refer us to this branch.

Please ignore the *umd.js file it VUE compile js, when doing a review.

This PR includes:

remove feedback popup and related(remove old angular js, PHP and related test and UI screen shoot)
plugin/feedback/**
add feedback question banner(VUE template, adapter, Php render, UI tests, integration Tests)
plugin/feedback/tests/**
extend icon with heart (icon-heart)
extend some date function with add month
extend get current user register date and last seen
extend modal adapter with validation, which does not close the modal.

Review

remove feedback links

update screen shot

add feedback question

update close to icon

diosmosis · 2021-11-03T05:11:43Z

@peterhashair will take a look at this now.

diosmosis · 2021-11-03T05:22:48Z

@peterhashair The AJAX issue is fixed by this:

https://github.com/matomo-org/matomo/blob/vue-comparisons/plugins/CoreHome/vue/src/AjaxHelper/AjaxHelper.ts#L154

There are a couple fixes like this in that branch, I can create a quick PR for that specific one.

I'm not sure why ./console vue:build Feedback doesn't work for you, it's working for me. Are you running it, then loading the app w/ development mode enabled? If so, can you check in the chrome sources whether the new file is actually loaded? If it's a file created with --watch, it will have a lot of eval(...)s.

diosmosis · 2021-11-03T05:24:50Z

Also, @sgiehl would you have time to checkout this branch and see if you can reproduce the vue:build Feedback vs vue:build --watch Feedback issue?

peterhashair · 2021-11-03T06:05:50Z

@diosmosis sorry, I may be confused you, the command itself works. But the interface looks different from watch.
Here is the screenshot, I guess it maybe related to <style scope>

diosmosis · 2021-11-03T19:37:07Z

@peterhashair oh I see, we're not supporting the use of styles from within Vue right now, instead we're still using the less files and core asset pipeline to define styling. This is mainly to keep the finished asset size down.

EDIT: If removing the style from the vue file doesn't change things, let me know.

remove in line css, change to less file

move translation key to plugin

remove feedback popup

peterhashair · 2021-11-03T22:38:35Z

@diosmosis yes it's the line <style scope> that causes the problem.

…ection # Conflicts: # plugins/Feedback/vue/dist/Feedback.umd.js # plugins/Feedback/vue/dist/Feedback.umd.min.js

update remove some test and feedback related

add draft tests

update feedback add php and UI tests

update test screen shots

add heart to demo page

add feedback back in extend the modal

…ollection # Conflicts: # plugins/CoreHome/vue/dist/CoreHome.umd.min.js

peterhashair · 2021-11-11T08:17:40Z

@sgiehl I think this is ready for another review 😀

sgiehl

Had another look at the changes. In general most stuff seems to work as expected now 👍
Left a couple of comments. Most of them are actually related to code cleanup and improvements.
Guess afterwards this will be ready to merge

core/Plugin/API.php

plugins/CoreHome/vue/src/CookieHelper/CookieHelper.adapter.ts

plugins/Feedback/API.php

plugins/CoreHome/vue/src/CookieHelper/CookieHelper.ts

plugins/Feedback/API.php

plugins/Feedback/Feedback.php

plugins/Feedback/vue/src/FeedbackQuestion/FeedbackQuestion.vue

sgiehl · 2021-11-11T15:00:56Z

plugins/Feedback/vue/src/FeedbackQuestion/FeedbackQuestion.vue

+      if (location.href.includes('forceFeedbackTest')) {
+        this.question = 0;
+        setCookie(cookieName, 0, 7);
+      } else {


I guess this one is to ensure the UI test always shows the same question? Guess this could be achieved also by setting the cookie in the UI test using page.setCookie

@sgiehl I tried set page.setCookie in the Test, but its return method was not found. Then I tried to add setCookies in tests/lib/screenshot-testing/support/page-renderer.js still no luck

You may need to add setCookie to the list of proxied methods here: https://github.com/matomo-org/matomo/blob/4.x-dev/tests/lib/screenshot-testing/support/page-renderer.js#L53
If that method is not proxied it will only be available through page.webpage.setCookie

Co-authored-by: Stefan Giehl <stefan@matomo.org>

update cookie and tests

…ollection # Conflicts: # plugins/CoreHome/vue/dist/CoreHome.umd.js # plugins/CoreHome/vue/dist/CoreHome.umd.min.js

test setcookie in tests

update tests

…ollection

update cookie test

extend setCookie in test

update elints format

test set cookie

…atomo into m-18251-feedback-collection

set cookie after goto URL

…atomo into m-18251-feedback-collection

int to string

update set cookie

update test URL

Peter Zhang added 3 commits November 2, 2021 11:50

remove feedback links

6b0f286

remove feedback links

update screen shot

7a8ad76

update screen shot

add feedback question

25bea17

add feedback question

peterhashair linked an issue Nov 3, 2021 that may be closed by this pull request

Collect feedback about what features people are using? #18251

Closed

21 tasks

peterhashair added this to the 4.6.0 milestone Nov 3, 2021

peterhashair added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Nov 3, 2021

Peter Zhang and others added 2 commits November 3, 2021 16:04

update close to icon

33adce0

update close to icon

built vue files

6a9f363

diosmosis mentioned this pull request Nov 3, 2021

[Vue] Make sure to use correct default params when building API requests in AjaxHelper.ts #18265

Merged

11 tasks

Peter Zhang added 3 commits November 4, 2021 11:05

remove in line css

3bf9449

remove in line css, change to less file

move translation key to plugin

4262af2

move translation key to plugin

remove feedback popup

b492e91

remove feedback popup

Peter Zhang added 4 commits November 4, 2021 14:49

Merge branch '4.x-dev' into m-18251-feedback-collection

5934428

Merge branch '4.x-dev' into m-18250-remove-review-popup

6c24c0e

Merge branch 'm-18250-remove-review-popup' into m-18251-feedback-coll…

fc01c6f

…ection # Conflicts: # plugins/Feedback/vue/dist/Feedback.umd.js # plugins/Feedback/vue/dist/Feedback.umd.min.js

update remove some test and feedback related

9c6dc00

update remove some test and feedback related

peterhashair mentioned this pull request Nov 4, 2021

remove feedback links #18253

Closed

11 tasks

peterhashair linked an issue Nov 4, 2021 that may be closed by this pull request

Remove review popup #18250

Closed

Peter Zhang added 6 commits November 4, 2021 17:18

add draft tests

8b6f849

add draft tests

update feedback add php and UI tests

bd3d910

update feedback add php and UI tests

update test screen shots

542fc10

update test screen shots

Update demo.twig

2fdabf1

add heart to demo page

add feedback back in extend the modal

869a0d7

add feedback back in extend the modal

Merge remote-tracking branch 'origin/4.x-dev' into m-18251-feedback-c…

3ebd09d

…ollection # Conflicts: # plugins/CoreHome/vue/dist/CoreHome.umd.min.js

Merge branch '4.x-dev' into m-18251-feedback-collection

b7047db

sgiehl requested changes Nov 11, 2021

View reviewed changes

Peter Zhang and others added 24 commits November 12, 2021 10:07

Update plugins/Feedback/Feedback.php

1b5b84f

Co-authored-by: Stefan Giehl <stefan@matomo.org>

Update plugins/Feedback/vue/src/FeedbackQuestion/FeedbackQuestion.vue

6e9bc95

Co-authored-by: Stefan Giehl <stefan@matomo.org>

Update plugins/Feedback/API.php

07fe4a2

Co-authored-by: Stefan Giehl <stefan@matomo.org>

Update plugins/Feedback/API.php

c110017

Co-authored-by: Stefan Giehl <stefan@matomo.org>

Update core/Plugin/API.php

654b667

Co-authored-by: Stefan Giehl <stefan@matomo.org>

update cookie and tests

4a28cd8

update cookie and tests

Merge remote-tracking branch 'origin/4.x-dev' into m-18251-feedback-c…

6b8d1a0

…ollection # Conflicts: # plugins/CoreHome/vue/dist/CoreHome.umd.js # plugins/CoreHome/vue/dist/CoreHome.umd.min.js

built vue files

a253043

test setcookie in tests

d12476f

test setcookie in tests

update tests

f7686df

update tests

update tests

86138a9

update tests

Merge remote-tracking branch 'origin/4.x-dev' into m-18251-feedback-c…

b3f12e8

…ollection

Update FeedbackQuestion_spec.js

05fd390

update cookie test

extend setCookie in test

0f41437

extend setCookie in test

update elints format

accacdd

update elints format

built vue files

e2aec94

test set cookie

7b2f4e5

test set cookie

Merge branch 'm-18251-feedback-collection' of github.com:matomo-org/m…

59327c5

…atomo into m-18251-feedback-collection

built vue files

9b62d54

Update FeedbackQuestion_spec.js

f3cee65

set cookie after goto URL

Merge branch 'm-18251-feedback-collection' of github.com:matomo-org/m…

bf37394

…atomo into m-18251-feedback-collection

Update FeedbackQuestion_spec.js

3b9a0d8

int to string

update set cookie

35a014e

update set cookie

update test URL

21d83bd

update test URL

peterhashair merged commit bf341b7 into 4.x-dev Nov 12, 2021

peterhashair deleted the m-18251-feedback-collection branch November 12, 2021 23:49

peterhashair mentioned this pull request Nov 14, 2021

Update Feedback Question banner #18308

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add feedback question, remove other related. #18262

add feedback question, remove other related. #18262

peterhashair commented Nov 3, 2021 •

edited

diosmosis commented Nov 3, 2021

diosmosis commented Nov 3, 2021

diosmosis commented Nov 3, 2021

peterhashair commented Nov 3, 2021

diosmosis commented Nov 3, 2021 •

edited

peterhashair commented Nov 3, 2021

peterhashair commented Nov 11, 2021

sgiehl left a comment

sgiehl Nov 11, 2021

peterhashair Nov 11, 2021

sgiehl Nov 11, 2021

add feedback question, remove other related. #18262

add feedback question, remove other related. #18262

Conversation

peterhashair commented Nov 3, 2021 • edited

Description:

Review

diosmosis commented Nov 3, 2021

diosmosis commented Nov 3, 2021

diosmosis commented Nov 3, 2021

peterhashair commented Nov 3, 2021

diosmosis commented Nov 3, 2021 • edited

peterhashair commented Nov 3, 2021

peterhashair commented Nov 11, 2021

sgiehl left a comment

Choose a reason for hiding this comment

sgiehl Nov 11, 2021

Choose a reason for hiding this comment

peterhashair Nov 11, 2021

Choose a reason for hiding this comment

sgiehl Nov 11, 2021

Choose a reason for hiding this comment

peterhashair commented Nov 3, 2021 •

edited

diosmosis commented Nov 3, 2021 •

edited