Skip to content
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

Migrate sitesmanager controller and twig templates to Vue components #18564

Merged
merged 703 commits into from Feb 7, 2022

Conversation

diosmosis
Copy link
Member

@diosmosis diosmosis commented Jan 4, 2022

Description:

This PR is based off of #18562.

Changes:

  • Make MatomoUrl.urlQuery/hashQuery public readonly so they can be watched.
  • Create ManageGlobalSettings vue component.
  • Create SiteManagement vue component.
  • related changes.

image

Review

…in the initial sites query if there was only one site in the entire matomo instance
@github-actions github-actions bot added the Stale The label used by the Close Stale Issues action label Jan 30, 2022
@diosmosis diosmosis added Do not close PRs with this label won't be marked as stale by the Close Stale Issues action and removed Stale The label used by the Close Stale Issues action labels Jan 30, 2022
Copy link
Contributor

@peterhashair peterhashair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you go setting->Websites->Manage->Add a new measurable->Roll-up
some of the options are missing. See screen shot

image

image

@diosmosis
Copy link
Member Author

@peterhashair if I understand correctly this should be fixed w/ the latest merge

@peterhashair
Copy link
Contributor

@diosmosis just had another look it seems the Roll-up still missing some parts. Do I need to merge another PR before this one?

@diosmosis
Copy link
Member Author

@peterhashair it looks the same to me on this branch as it does on 4.x-dev. What is missing for you?

@peterhashair
Copy link
Contributor

peterhashair commented Feb 2, 2022

@diosmosis I think some of the premium plugins append additional fields to the Site Manager. The first video is the current PR. The second is the 4.x-dev

screen-capture.mp4
expected.mp4

@diosmosis
Copy link
Member Author

@peterhashair this is what I see locally on this branch:

(adding)
localhost_index php_module=SitesManager action=index idSite=1 period=range date=previous30

(editing)
localhost_index php_module=SitesManager action=index idSite=1 period=range date=previous30 (1)

Can you pull, rebuild and clear your cache?

@peterhashair
Copy link
Contributor

@diosmosis ah, I did all those, but still no luck. I guess it may be some package missing. Will try more build.

@sgiehl
Copy link
Member

sgiehl commented Feb 2, 2022

@diosmosis another issue that might be good to fix in that one:

  • It's possible to open the form for adding a measurable multiple times. But clicking a cancel button closes all of them at once. Imho we should simply close the previous form before opening a new one

@diosmosis
Copy link
Member Author

@sgiehl it seems like the existing code was meant to hide the top button bar while a site is being edited/added, but it doesn't really work. I made it work and changed it to disable the bar while a site is being edited/added since it looked odd to simply remove it.

Allowing multiple add modals seemed more complicated.

@sgiehl
Copy link
Member

sgiehl commented Feb 2, 2022

@diosmosis hiding the button to add another one sounds fine as well 👍

Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had another quick look through the code and the UI. Seems to work as expected.

@sgiehl sgiehl merged commit 80a47f8 into 4.x-dev Feb 7, 2022
@sgiehl sgiehl deleted the vue-sitesmanager-2 branch February 7, 2022 11:32
@justinvelluppillai justinvelluppillai changed the title [Vue] migrate sitesmanager controller and twig templates to Vue components Migrate sitesmanager controller and twig templates to Vue components Feb 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Do not close PRs with this label won't be marked as stale by the Close Stale Issues action Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants