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

Individual Workflow Template Page / Part I #2595

Conversation

SchnapsterDog
Copy link
Contributor

@SchnapsterDog SchnapsterDog commented Dec 23, 2021

N8N-2677 - Created Vuex Templates, Responses, Interfaces, TemplateView, MarkdownViewer Component, Small Components in Design System, TemplateDetails Component

…mplates, Responses, Interfaces, TemplateView, MarkdownViewer Component, Small Components in Design System, TemplateDetails Component
@SchnapsterDog SchnapsterDog self-assigned this Dec 23, 2021
@ivov ivov added ui Enhancement in /editor-ui or /design-system n8n team Authored by the n8n team labels Dec 23, 2021
@ivov ivov changed the title N8N-2677 Individual Workflow Template Page / Part I - Created Vuex Templates, Responses, Interfaces, TemplateView, MarkdownViewer Component, Small Components in Design System, TemplateDetails Component Individual Workflow Template Page / Part I Dec 23, 2021
Copy link
Contributor

@mutdmour mutdmour left a comment

Choose a reason for hiding this comment

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

nice job. took a quick look and just add a few comments to work on.
Please make sure to delete the empty NodeIcon files

<template>
<div :class="$style.wrapper" @click="$router.go(-1)">
<font-awesome-icon :class="$style.icon" icon="arrow-left" />
<n8n-text size="medium" color="text-base">Go back</n8n-text>
Copy link
Contributor

Choose a reason for hiding this comment

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

locale.baseText('goBack')

Comment on lines 18 to 23
width: fit-content;
margin-right: 4px;
padding: 3px 4px;
background-color: #dbdfe7;
border-radius: var(--border-radius-base);
font-size: 11px;
Copy link
Contributor

Choose a reason for hiding this comment

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

please use css variables wherever you can

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done & updated :) Only for this background-color and font-size: 11px; we do not have css variables.

Comment on lines 21 to 22
margin-right: 8px;
color: #C6C8D0;
Copy link
Contributor

Choose a reason for hiding this comment

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

css variables please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done for the margin-right. Color is not present in the design system. :/

<div ref="editor" :class="$style.markdown" v-html="getContent" />
</template>

<script lang="ts">
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this not a component in the design system?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved

},
methods: {
numberFormater(num: number) {
return Math.abs(num) > 999
Copy link
Contributor

Choose a reason for hiding this comment

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

can we move this to a helper file since we will be reusing this? also i think it needs to support m

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

const module: Module<ITemplateState, IRootState> = {
namespaced: true,
state: {
template: {},
Copy link
Contributor

Choose a reason for hiding this comment

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

in state we should support having multiple templates so that we can easily extend this to support search page...
so this should be mapping template ids to template objects

templates: {[id: string]: ITemplate}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

Comment on lines +832 to +836
"buttons": {
"goBackButton": "Go back",
"useThisWorkflowButton": "Use this workflow"
},
"details": {
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure if we need this nesting here. @ivov can you give feedback here? this is the expected view
Screenshot 2021-12-24 at 13 23 43

Comment on lines +85 to +87
setTimeout(() => {
this.$router.go(-1);
}, 2000);
Copy link
Contributor

Choose a reason for hiding this comment

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

might not be necessary here to go back

@SchnapsterDog SchnapsterDog marked this pull request as ready for review December 28, 2021 10:21
@@ -37,7 +37,7 @@ export const BREAKPOINT_XL = 1920;


// templates
export const TEMPLATES_BASE_URL = `https://api.n8n.io/`;
export const TEMPLATES_BASE_URL = `https://api-staging.n8n.io/`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reminder to change this. Added api-staging for testing purpose

@mutdmour
Copy link
Contributor

closed in favor of merging all template branches together

@mutdmour mutdmour closed this Jan 20, 2022
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.

None yet

3 participants