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.

Sign up for GitHub

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: Add Creator hub link to Templates page #7721

Merged
merged 6 commits into from
Nov 20, 2023
Merged

feat: Add Creator hub link to Templates page #7721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1f083e3
feat: Add Creator hub link to Templates page
MiloradFilipovic Nov 15, 2023
dda4a56
👌 Updating button copy
MiloradFilipovic Nov 15, 2023
d3d29e5
👌 Updating button component to support anchor tag, removing unused i1…
MiloradFilipovic Nov 16, 2023
a9535ce
✔️ Updating test snapshots
MiloradFilipovic Nov 16, 2023
4a2f4f9
Merge branch 'master' into ADO-1406-add-link-to-creator-hub
MiloradFilipovic Nov 20, 2023
9c83495
👌 Refactoring button component prop definition, removing unused type …
MiloradFilipovic Nov 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .../design-system/src/components/N8nActionBox/__tests__/__snapshots__/ActionBox.spec.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ exports[`N8NActionBox > should render correctly 1`] = `
<div class=\\"description\\">
<n8n-text-stub color=\\"text-base\\" bold=\\"false\\" size=\\"medium\\" compact=\\"false\\" tag=\\"span\\"></n8n-text-stub>
</div>
<n8n-button-stub label=\\"Do something\\" type=\\"primary\\" size=\\"large\\" loading=\\"false\\" disabled=\\"false\\" outline=\\"false\\" text=\\"false\\" block=\\"false\\" active=\\"false\\" square=\\"false\\"></n8n-button-stub>
<n8n-button-stub label=\\"Do something\\" type=\\"primary\\" size=\\"large\\" loading=\\"false\\" disabled=\\"false\\" outline=\\"false\\" text=\\"false\\" block=\\"false\\" active=\\"false\\" square=\\"false\\" element=\\"button\\"></n8n-button-stub>
<!--v-if-->
</div>"
`;
23 changes: 20 additions & 3 deletions packages/design-system/src/components/N8nButton/Button.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
<template>
<button
<component
:is="element"
Copy link
Collaborator

@tomi tomi Nov 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think this would be clearer if the <a> "button" was a separate component for couple of reasons. 1) when I use a component named Button I'd imagine it's a <button> 2) now we have props like href which doesn't make sense for a <button>.

So maybe we could have a N8nLinkButton or so that looks like a button, but is an <a> element with <a> props. These could have the same base class if DRY is a concern.

E: Then again I kinda like this approach as well. But maybe in that case we could rename the link prop to a as prop that is button by default and can be changed to a. And make sure href attribute is not added unless the element is <a>. Anyways, both are fine, you can make the call 😊

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, DRY was my main concern here since it looks like the Button component still has a lot for styling in the component itself and didn't want to add more scope to this by refactoring it. So I decided to take this route (taken from other frameworks like Quasar or Element UI).
Agree about the href validation will add that and will change the link prop to be string with values button, a but I think elemnent is more clear name for it then as

:class="classes"
:disabled="isDisabled"
:aria-disabled="ariaDisabled"
:aria-busy="ariaBusy"
:href="href"
aria-live="polite"
v-bind="$attrs"
>
Expand All @@ -14,13 +16,13 @@
<span v-if="label || $slots.default">
<slot>{{ label }}</slot>
</span>
</button>
</component>
</template>

<script setup lang="ts">
import N8nIcon from '../N8nIcon';
import N8nSpinner from '../N8nSpinner';
import { useCssModule, computed, useAttrs } from 'vue';
import { useCssModule, computed, useAttrs, watchEffect } from 'vue';

const $style = useCssModule();
const $attrs = useAttrs();
Expand Down Expand Up @@ -72,6 +74,21 @@ const props = defineProps({
type: Boolean,
default: false,
},
element: {
type: String,
default: 'button',
validator: (value: string) => ['button', 'a'].includes(value),
},
href: {
type: String,
required: false,
},
});

watchEffect(() => {
if (props.element === 'a' && !props.href) {
console.error('n8n-button:href is required for link buttons');
}
});

const ariaBusy = computed(() => (props.loading ? 'true' : undefined));
Expand Down
2 changes: 2 additions & 0 deletions packages/editor-ui/src/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -648,3 +648,5 @@ export const DRAG_EVENT_DATA_KEY = 'nodesAndConnections';

export const NOT_DUPLICATABE_NODE_TYPES = [FORM_TRIGGER_NODE_TYPE];
export const UPDATE_WEBHOOK_ID_NODE_TYPES = [FORM_TRIGGER_NODE_TYPE];

export const CREATOR_HUB_URL = 'https://creators.n8n.io/hub';
2 changes: 1 addition & 1 deletion packages/editor-ui/src/plugins/i18n/locales/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1697,7 +1697,7 @@
"templates.collectionsNotFound": "Collection could not be found",
"templates.connectionWarning": "⚠️ There was a problem fetching workflow templates. Check your internet connection.",
"templates.heading": "Workflow templates",
"templates.newButton": "New blank workflow",
"templates.shareWorkflow": "Share template",
"templates.noSearchResults": "Nothing found. Try adjusting your search to see more.",
"templates.searchPlaceholder": "Search workflows",
"templates.workflows": "Workflows",
Expand Down
14 changes: 7 additions & 7 deletions packages/editor-ui/src/views/TemplatesSearchView.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,11 @@
<div :class="$style.button">
<n8n-button
MiloradFilipovic marked this conversation as resolved.
Show resolved Hide resolved
size="large"
:label="$locale.baseText('templates.newButton')"
MiloradFilipovic marked this conversation as resolved.
Show resolved Hide resolved
@click="openNewWorkflow"
type="secondary"
element="a"
:href="creatorHubUrl"
:label="$locale.baseText('templates.shareWorkflow')"
target="_blank"
/>
</div>
</div>
Expand Down Expand Up @@ -90,7 +93,7 @@ import type {
} from '@/Interface';
import type { IDataObject } from 'n8n-workflow';
import { setPageTitle } from '@/utils';
import { VIEWS } from '@/constants';
import { CREATOR_HUB_URL, VIEWS } from '@/constants';
import { debounceHelper } from '@/mixins/debounce';
import { useSettingsStore } from '@/stores/settings.store';
import { useUsersStore } from '@/stores/users.store';
Expand Down Expand Up @@ -132,6 +135,7 @@ export default defineComponent({
search: '',
searchEventToTrack: null as null | ISearchEvent,
errorLoadingWorkflows: false,
creatorHubUrl: CREATOR_HUB_URL as string,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the type cast needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like it's not. Typescript was complaining it wasn't able to infer type for it but seems like it was just my editor acting weirdly

};
},
computed: {
Expand Down Expand Up @@ -224,10 +228,6 @@ export default defineComponent({
this.searchEventToTrack = null;
}
},
openNewWorkflow() {
this.uiStore.nodeViewInitialized = false;
void this.$router.push({ name: VIEWS.NEW_WORKFLOW });
},
onSearchInput(search: string) {
this.loadingWorkflows = true;
this.loadingCollections = true;
Expand Down
Loading