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(editor): VariablesView Reskin - Add Filters for missing values #12611

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

Conversation

r00gm
Copy link
Contributor

@r00gm r00gm commented Jan 15, 2025

Summary

Table reskin

image
image

Filters

image
image

Related Linear tickets, Github issues, and Community forum posts

PAY-2459

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 Jan 15, 2025
@Tuukkaa
Copy link

Tuukkaa commented Jan 16, 2025

Hey, I noticed that the validation message gets cut off in the Key field. Could we make the first column (Key) of the table wider, so that it has the same width than the second column (Value)?

Also, let's update the tag and filter to "Needs attention" to match what was discussed in Linear

image

@r00gm
Copy link
Contributor Author

r00gm commented Jan 16, 2025

Hey, I noticed that the validation message gets cut off in the Key field. Could we make the first column (Key) of the table wider, so that it has the same width than the second column (Value)?

Also, let's update the tag and filter to "Needs attention" to match what was discussed in Linear

image

I missed the "Needs attention" thanks!!

About the column size, what i can do is only set the width of the actions column and the rest will auto adjust to the content. wdyt @Tuukkaa ?

image

Copy link

codecov bot commented Jan 16, 2025

Codecov Report

Attention: Patch coverage is 96.38158% with 11 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
packages/editor-ui/src/views/VariablesView.vue 94.14% 11 Missing ⚠️

📢 Thoughts on this report? Let us know!

@Tuukkaa
Copy link

Tuukkaa commented Jan 16, 2025

@r00gm can we adjust the table so that the Key, Value and Usage syntax always take the same width, and the actions take the least possible space? With CSS grid it would be something like: 1fr 1fr 1fr fit-content.

@r00gm
Copy link
Contributor Author

r00gm commented Jan 16, 2025

@r00gm can we adjust the table so that the Key, Value and Usage syntax always take the same width, and the actions take the least possible space? With CSS grid it would be something like: 1fr 1fr 1fr fit-content.

done!
image

@Tuukkaa
Copy link

Tuukkaa commented Jan 16, 2025

Hey Raul, I got a confused between the Variables and Credentials table. I actually think that in this variables table it would be better to:

  1. Name the tag as "Value missing" instead of "Needs attention".
  2. Show the tag inside the Value column
  3. Show a "Value missing" filter inside the filter dropdown

I updated the Figma design here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the table is only used in the Variables Page. Reskin asked by design

@@ -50,6 +50,7 @@ export interface Props {
inactiveColor?: string;
teleported?: boolean;
tagSize?: 'small' | 'medium' | 'large';
autosize?: boolean | { minRows: number; maxRows: number };
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this component replaces packages/editor-ui/src/components/VariablesRow.vue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

implemented testing pinia

test filters
test sorting
test CRUD

@r00gm r00gm changed the title Pay 2459 add filters to variables to highlight empty variables to feat(editor): VariablesView Reskin - Add Filters for missing values Jan 16, 2025
@r00gm r00gm marked this pull request as ready for review January 16, 2025 16:05
@r00gm r00gm requested a review from cstuncsik January 16, 2025 16:06
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