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

[Vue] rename PluginSetting component to GroupedSetting component and refactor condition handling #18628

Merged
merged 3 commits into from Jan 18, 2022

Conversation

diosmosis
Copy link
Member

Description:

Changes:

  • rename PluginSetting component to GroupedSetting (changed :plugin-name to :group-name and allow it to be optional)
    This component encapsulates the Vue side handling of the formField.condition parameter in angularjs, but in a way that doesn't require FormField to know about every other setting/setting value. It's used in measurable settings and tag manager parameters.
  • refactored condition handling to take place in GroupedSetting instead of FormField to reduce the amount of work FormField does and so we don't have to pass a dynamic function as a property to FormField and so FormField doesn't have to show/hide itself. BC w/ angularjs should still be maintained.

@sgiehl @peterhashair it would be good to review this one before doing the sitesmanager PRs.

Review

…d refactor angularjs condition handling in vue
@diosmosis diosmosis added the Needs Review PRs that need a code review label Jan 15, 2022
@diosmosis diosmosis added this to the 4.7.0 milestone Jan 15, 2022
@diosmosis
Copy link
Member Author

had a better idea, will push a change soon

@diosmosis diosmosis marked this pull request as draft January 16, 2022 20:01
…e computed properties + get to work in the UI
@diosmosis diosmosis marked this pull request as ready for review January 16, 2022 23:19
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.

Looks good to me. See if anyone has any updates.

@diosmosis diosmosis merged commit 165be4f into 4.x-dev Jan 18, 2022
@diosmosis diosmosis deleted the vue-grouped-setting branch January 18, 2022 00:33
@diosmosis diosmosis modified the milestones: 4.7.0, 4.8.0 Jan 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants