@peterhashair opened this Pull Request on November 3rd 2021 Contributor

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:

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

Review

@diosmosis commented on November 3rd 2021 Member

@peterhashair will take a look at this now.

@diosmosis commented on November 3rd 2021 Member

@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 commented on November 3rd 2021 Member

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 commented on November 3rd 2021 Contributor

@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>
image

@diosmosis commented on November 3rd 2021 Member

@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.

@peterhashair commented on November 3rd 2021 Contributor

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

@peterhashair commented on November 8th 2021 Contributor

@bx80 good spot for the cancel and reopen the bug, fixed it :)

@peterhashair commented on November 8th 2021 Contributor

@sgiehl yeah, that logic on banner show code is a little bit confusing. Just update it, hopefully, it makes sense now. 😀

@bx80 commented on November 8th 2021 Contributor

@peterhashair One minor thing I noticed - the post question 'thanks' dialog doesn't have a 'Close' or 'Ok' button. Not sure if that is intentional?
image

@peterhashair commented on November 9th 2021 Contributor

@bx80 yes, that makes sense, added a close button to the finished screen.

@peterhashair commented on November 9th 2021 Contributor

@sgiehl sorry to bother you, do you want to do a final check on that one 😀

@peterhashair commented on November 10th 2021 Contributor

there could be a bug, because I didn't record the question they answers, after 6 months, they maybe answer the same question again.

@peterhashair commented on November 11th 2021 Contributor

@sgiehl I think this is ready for another review 😀

This Pull Request was closed on November 12th 2021
Powered by GitHub Issue Mirror