[Vue] migrate siteselector directive and quick-access directive #18292

diosmosis · 2021-11-11T05:59:42Z

Description:

This PR is based off of #18225.

This PR has a breaking change to createAngularAdapter() so other Vue PRs in progress may need to be modified (unless they are merged before this, then I'll fix the conflicts here).

Changes:

Add typescript types for mousetrap library.
deprecate autocomplete-matched as it's trivial to implement in siteselector and is not used anywhere else.
in broadcast.propagateNewPage() add extra parameter for supplying the entire new URL to load. in this case, it does not build the URL using other parameters.
add a MatomoUrl.updateUrl() method that reloads the entire URL (search + hash)
make sure promise returned by AjaxHelper.fetch has an abort() method
in createAngularAdapter() add vue instance to the event handler functions and postCreate(). this is so ngModel could be connected to v-model in Vue.
migrated the siteselector directive.
migrated the quick-access directive.
migrated the siteselector-model service as the SitesStore. This is shared between quick-access and siteselector. The only state that's shared is the initial list of sites that is loaded once, unlike currently where the list of currently displayed sites is also shared between the two directives.
in createAngularAdapter() allow specifying a require: field to the angularjs directive that is created.
add the ability to transform values when binding from angularjs => vue (for example, converting values to booleans, as in noclear for siteselector)
disable webpack asset size hints (since our assets are already bigger at the end than what is recommended by webpack).
when changing sites, removed the showAjaxLoading = false parameter so MatomoUrl.updateUrl() will stay simple. Doesn't seem to have much of an effect IMO so I think it's ok.

Review

…ty method (all untested)

…se v-model, add event parameters to createAngularAdapter, allow translate to use variadic args or one string array + rebuild

…o on 4.x-dev)

…s in between nodes while vue 3 does not

…n to not work

…cript tests.

…ndling to just reset back to the previous date since it was easier in vue to do that)

peterhashair

That works for me perfectly, maybe still need to do resolve conflicts, also just a tiny suggestion maybe on key up autocomplete event maybe we need a debounce.

diosmosis · 2021-11-15T22:16:34Z

plugins/CoreHome/vue/src/QuickAccess/QuickAccess.vue

+      } else if (isEscKey && areSearchResultsDisplayed) {
+        this.deactivateSearch();
+      } else {
+        setTimeout(() => {


There wasn't one before but we can add one.

diosmosis · 2021-11-15T22:16:34Z

plugins/CoreHome/vue/src/QuickAccess/QuickAccess.vue

+      } else if (isEscKey && areSearchResultsDisplayed) {
+        this.deactivateSearch();
+      } else {
+        setTimeout(() => {


There wasn't one before but we can add one.

peterhashair · 2021-11-15T22:29:43Z

@diosmosis also I got an question for function naming.
like that one, do we need a camel case?

export { default as Menudropdown } from './Menudropdown/Menudropdown.vue';

diosmosis · 2021-11-15T22:39:19Z

I got an question for function naming. like that one, do we need a camel case?

Yes, that would probably be better 👍

diosmosis · 2021-11-16T23:47:21Z

converted to draft so it won't get merged until after the 4.6.0 release

justinvelluppillai · 2021-11-17T00:12:22Z

converted to draft so it won't get merged until after the 4.6.0 release

PRs merged to 4.x-dev won't be released in 4.6.0

diosmosis · 2021-11-17T01:46:21Z

@justinvelluppillai ok, removing draft status

peterhashair

looks good to me.

sgiehl · 2021-11-17T14:37:35Z

@diosmosis I'm currently doing some local testing of the changes. Will leave some more comments when I find something else.

It seems the search results in site selector are not escaped correctly. I have a website with the name <a href="#">?</a>, it displays correctly when listed on opening the siteselector. But when searching, it is only displayed as ?, the html tags seems to be inserted. So maybe a script tag might evaluate here as well.

Note: In quick search that seems to work correctly.
When I type something into the search box in site selector and then click on the input field again, the field either looses it's content (if the request is still pending), or it sometimes even disappears (when the request had finished)
Pressing the f key highlights the quick search visually, but doesn't give the input box the focus anymore
Pressing the w key opens and highlights the site selector visually, but doesn't give the input box the focus anymore
Unrelated to the changes here. But pressing d doesn't open the date picker any longer. Maybe that's due to other vue changes before

diosmosis · 2021-11-17T22:14:54Z

So maybe a script tag might evaluate here as well.

It's sanitized so dangerous HTML is removed. The other html I guess isn't being escaped properly.

diosmosis · 2021-11-17T23:01:45Z

It seems the search results in site selector are not escaped correctly. I have a website with the name ?, it displays correctly when listed on opening the siteselector. But when searching, it is only displayed as ?, the html tags seems to be inserted. So maybe a script tag might evaluate here as well.

Can you check again? this is working for me:

When I type something into the search box in site selector and then click on the input field again, the field either looses it's content (if the request is still pending), or it sometimes even disappears (when the request had finished)

Looks like there's a bug in the original code. Should be fixed in the vue PR now.

Pressing the f key highlights the quick search visually, but doesn't give the input box the focus anymore

Fixed.

Pressing the w key opens and highlights the site selector visually, but doesn't give the input box the focus anymore

Fixed.

Unrelated to the changes here. But pressing d doesn't open the date picker any longer. Maybe that's due to other vue changes before

Found the issue, will fix in PR to next_release.

sgiehl · 2021-11-18T08:33:49Z

@diosmosis hm. funny. It works when searching for ? for me as well. But when searching for any letter that actually isn't in the site's name, it doesn't.

diosmosis · 2021-11-18T23:09:33Z

@sgiehl should be fixed. Looks like the issue only occurred when searching for html entity letters (eg, searching for gt which in the backend will match <img onsrc...).

sgiehl

works now 👍

diosmosis added 30 commits October 16, 2021 00:36

migrating RateFeature and ReviewLinks + adding AjaxHelper.fetch utili…

f7728e4

…ty method (all untested)

get ratefeature component to work, modify matomodialog component to u…

fb92176

…se v-model, add event parameters to createAngularAdapter, allow translate to use variadic args or one string array + rebuild

remove ratefeature angularjs files

57ff557

rebuild + make vue mapping property optional in createANgularJsAdapter

d11d768

migrate enrichedheadline and get to work

ea0fb24

fix test

e922566

fix translate

38a3523

fix another translate issue & migrate contentblock directive

9d95c6d

fix anchor links, not including the "/" causes angularjs to fail (als…

6f208b5

…o on 4.x-dev)

update expected screenshots

a954411

fix ui test

5f2ef78

fix some test failures

6d95fa1

fix nested transclude issue

b13348f

remove content block files

8f0e50e

fix icon spacing that occurs due to angularjs inserting empty comment…

d1b8563

…s in between nodes while vue 3 does not

update some screenshots

108ea71

update screenshot (actually fixes an alignment issue)

5cc54d8

update screenshot

17a71fb

first pass at converting comparisons service/component

48e435d

get new code to build and load without error in the UI

39e8913

debugging

440ec9e

getting basic functionaltiy to work

a5b83dd

Update _dataTable.twig

276bec3

fix UI test failure + URL encoding/angularjs issue causing back butto…

2dbdf3c

…n to not work

fix order of operations issue

1bf469d

built vue files

33b9b65

using ref in setup() is not needed to access this.$refs

7715802

Convert comparisons service angularjs tests to comparison store types…

26e0eb1

…cript tests.

migrate piwik-date-picker directive

3b809f8

migrate date range picker component (changed invalid date in input ha…

385187a

…ndling to just reset back to the previous date since it was easier in vue to do that)

built vue files

49c179b

peterhashair reviewed Nov 15, 2021

View reviewed changes

diosmosis commented Nov 15, 2021

View reviewed changes

diosmosis marked this pull request as draft November 16, 2021 23:47

apply review feedback

ba23a4d

Merge branch '4.x-dev' into vue-siteselector

5851529

diosmosis marked this pull request as ready for review November 17, 2021 01:46

peterhashair approved these changes Nov 17, 2021

View reviewed changes

diosmosis added 2 commits November 17, 2021 14:27

get auto clearing behavior to work in site selector

73ae815

fix a couple more bugs

da7f33d

rebuild vue

8348784

sgiehl modified the milestones: 4.6.0, 4.7.0 Nov 18, 2021

escape htmle entities in site name

a798d24

diosmosis mentioned this pull request Nov 19, 2021

[Vue] migrate piwik-field and related directives #18352

Merged

11 tasks

tweak

fa451ce

sgiehl removed the Needs Review PRs that need a code review label Nov 19, 2021

sgiehl approved these changes Nov 19, 2021

View reviewed changes

sgiehl merged commit 5ae81f8 into 4.x-dev Nov 19, 2021

sgiehl deleted the vue-siteselector branch November 19, 2021 14:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Vue] migrate siteselector directive and quick-access directive #18292

[Vue] migrate siteselector directive and quick-access directive #18292

diosmosis commented Nov 11, 2021 •

edited

peterhashair left a comment

diosmosis Nov 15, 2021

diosmosis Nov 15, 2021

peterhashair commented Nov 15, 2021

diosmosis commented Nov 15, 2021

diosmosis commented Nov 16, 2021

justinvelluppillai commented Nov 17, 2021

diosmosis commented Nov 17, 2021

peterhashair left a comment

sgiehl commented Nov 17, 2021 •

edited

diosmosis commented Nov 17, 2021

diosmosis commented Nov 17, 2021

sgiehl commented Nov 18, 2021

diosmosis commented Nov 18, 2021

sgiehl left a comment

[Vue] migrate siteselector directive and quick-access directive #18292

[Vue] migrate siteselector directive and quick-access directive #18292

Conversation

diosmosis commented Nov 11, 2021 • edited

Description:

Review

peterhashair left a comment

Choose a reason for hiding this comment

diosmosis Nov 15, 2021

Choose a reason for hiding this comment

diosmosis Nov 15, 2021

Choose a reason for hiding this comment

peterhashair commented Nov 15, 2021

diosmosis commented Nov 15, 2021

diosmosis commented Nov 16, 2021

justinvelluppillai commented Nov 17, 2021

diosmosis commented Nov 17, 2021

peterhashair left a comment

Choose a reason for hiding this comment

sgiehl commented Nov 17, 2021 • edited

diosmosis commented Nov 17, 2021

diosmosis commented Nov 17, 2021

sgiehl commented Nov 18, 2021

diosmosis commented Nov 18, 2021

sgiehl left a comment

Choose a reason for hiding this comment

diosmosis commented Nov 11, 2021 •

edited

sgiehl commented Nov 17, 2021 •

edited