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

feat(plugin-form): disable scoped plugin toggle #1816

Merged
merged 6 commits into from
Dec 5, 2024

Conversation

TT1228
Copy link
Contributor

@TT1228 TT1228 commented Nov 26, 2024

Summary

KM-621

Instead of hiding scope selection group under scoped plugin, the source scope is fixed and global/scoped checkbox group is greyed out with explanation(by passing a new prop disableScopeSelection). This keeps UI consistency and provide easier configuration of scoped plugins.

Slack: https://kongstrong.slack.com/archives/C03NRECFJPM/p1732126368893859?thread_ts=1729624945.925459&cid=C03NRECFJPM

KM
disableScopeSelection is not passed, hideScopeSelection is kept.
截屏2024-11-28 上午9 42 37
KM Dev Portal
截屏2024-11-28 上午9 43 07

GM
hideScopeSelection is replaced by disableScopeSelection.
截屏2024-11-28 上午9 44 02

GM preview link: https://pr-5021--gateway-manager.cloud-preview.konghq.tech/gateway-manager

@TT1228 TT1228 requested review from LukeSwierlik, QueaT-kong and a team as code owners November 26, 2024 06:32
@@ -99,6 +114,9 @@ export default {
entity() {
return this.schema.entity
},
scope() {
return this.entity.replace('_',' ').replace(/s$/,'')
Copy link
Contributor

Choose a reason for hiding this comment

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

What if we add some entity with three words in the name, or the name ends with y? I think it's clearer to create a string mapping instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

On a second thought, should the scope value also be an i18n key? Maybe we can use entity as that key directly?

v-for="(option, i) in schema.fields"
:key="i"
class="option-group"
:class="{ 'radio-diabled': $props.disabled }"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
:class="{ 'radio-diabled': $props.disabled }"
:class="{ 'radio-disabled': $props.disabled }"

@@ -157,6 +178,16 @@ export default {
flex-direction: row;
gap: $kui-space-80;
}

.radio-diabled {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.radio-diabled {
.radio-disabled {


input {
cursor: not-allowed;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We can group the above rulesets.

@@ -443,7 +443,8 @@ const defaultFormSchema: DefaultPluginsSchemaRecord = reactive({
},
// plugin scoping
selectionGroup: {
type: !props.hideScopeSelection ? 'selectionGroup' : props.hideScopeSelection || (formType.value === EntityBaseFormType.Create && props.config.entityId) ? 'foreign' : 'selectionGroup',
type: 'selectionGroup',
disabled: !props.hideScopeSelection ? false : props.hideScopeSelection || (formType.value === EntityBaseFormType.Create && props.config.entityId) ? true : false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's reverse the condition for better readability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. When i copied those code, i already found it weird that || (formType.value === EntityBaseFormType.Create && props.config.entityId) is totally not working

@TT1228 TT1228 force-pushed the feat/KM-621-new-scoped-plugin-behavior branch from fb59b8f to 5b29f32 Compare November 27, 2024 09:33
@Leopoldthecoder
Copy link
Member

Note: let's merge this PR after 3.9 code freeze.

@TT1228 TT1228 force-pushed the feat/KM-621-new-scoped-plugin-behavior branch from 5b29f32 to 51d0701 Compare December 4, 2024 10:11
@TT1228 TT1228 force-pushed the feat/KM-621-new-scoped-plugin-behavior branch from 51d0701 to 2c2d9bf Compare December 5, 2024 09:07
@Leopoldthecoder Leopoldthecoder merged commit def603a into main Dec 5, 2024
12 checks passed
@Leopoldthecoder Leopoldthecoder deleted the feat/KM-621-new-scoped-plugin-behavior branch December 5, 2024 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants