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

refactor(editor): Migrate components to composition API #11497

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

mutdmour
Copy link
Contributor

@mutdmour mutdmour commented Nov 1, 2024

Summary

Migrate the following components to composition API:

  • CommunityPackageCard.vue
  • InviteUserModal.vue
  • RunDataJsonActions.vue
  • EventDestinationCard.ee.vue
  • TemplateCard.vue
  • TemplateFilters.vue
  • TemplateInfoCard.vue
  • SettingsApi.vue
  • SettingsLogStreaming.vue

Review / Merge checklist

  • PR title and summary are descriptive. (conventions)
  • Docs updated or follow-up ticket created.
  • Tests included.
  • PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

@n8n-assistant n8n-assistant bot added n8n team Authored by the n8n team ui Enhancement in /editor-ui or /design-system labels Nov 1, 2024
function onUpdateClick() {
if (!props.communityPackage) return;
openCommunityPackageUpdateConfirmModal(props.communityPackage.packageName);
}
</script>

<template>
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 replace the $locale in the template for i18n

return parsed;
}
const usersStore = useUsersStore();
const settingsStore = useSettingsStore();
Copy link
Contributor

Choose a reason for hiding this comment

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

});
const enabledButton = computed((): boolean => {
return emailsCount.value >= 1;
});
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: let's add spaces

return settingsStore.isEnterpriseFeatureEnabled[EnterpriseEditionFeature.AdvancedPermissions];
});

function validateEmails(value: string | number | boolean | null | undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpit: We have a mix of arrow functions assigned to a variable and normal functions. Can we stick to one? ideally the former

}
}
return parsed;
}
</script>

<template>
Copy link
Contributor

Choose a reason for hiding this comment

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

replace $locale from template with i18n

const { isPublicApiEnabled, isSwaggerUIEnabled, publicApiPath, publicApiLatestVersion } =
settingsStore;
const isTrialing = computed((): boolean => {
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: const isTrialing = computed(() => cloudPlanStore.userIsTrialing)

Copy link
Contributor

Choose a reason for hiding this comment

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

Same with isLoadingCloudPlans

const isTrialing = computed((): boolean => {
return cloudPlanStore.userIsTrialing;
});
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: spaces between functions and maybe arrow functions instead. Also, keep order suggested in the document

@@ -185,14 +171,14 @@ export default defineComponent({
<div :class="$style.hint">
<n8n-text size="small">
{{
$locale.baseText(`settings.api.view.${swaggerUIEnabled ? 'tryapi' : 'more-details'}`)
$locale.baseText(`settings.api.view.${isSwaggerUIEnabled ? 'tryapi' : 'more-details'}`)
Copy link
Contributor

Choose a reason for hiding this comment

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

remove $locale and use i18n instead

},
});
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

remove $locale from template and use i18n

await this.getDestinationDataFromBackend();
const eventBus = createEventBus();
const disableLicense = ref(false);
const allDestinations = ref<MessageEventBusDestinationOptions[]>([]);
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: keep the order suggested in the document

@RicardoE105 RicardoE105 changed the title refactor: Move components to composables (no-changelog) refactor(editor): Migrate components to composition API Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
n8n team Authored by the n8n team ui Enhancement in /editor-ui or /design-system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants