-
Notifications
You must be signed in to change notification settings - Fork 27
feat(variables): add variables tabs at environment scope #2367
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
base: new-navigation
Are you sure you want to change the base?
Conversation
|
Qovery can create a Preview Environment for this PR.
This comment has been generated from Qovery AI 🤖.
|
…nd update UI components
|
/qovery preview 15d69f24-9bc1-4a8d-80fe-d1bb1b2bcd00 |
RemiBonnet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done @TheoGrandin74 !
I added few small comments for structure and testing
| import { type APIVariableScopeEnum } from 'qovery-typescript-axios' | ||
| import { type UseFormSetValue } from 'react-hook-form' | ||
|
|
||
| export function changeScopeForAll( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The extension of the file should be .ts and not .tsx
Could you add a unit test for it? (small one)
| @@ -0,0 +1,13 @@ | |||
| export function deleteEntry( | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
| @@ -0,0 +1,24 @@ | |||
| import { APIVariableScopeEnum } from 'qovery-typescript-axios' | |||
|
|
|||
| export function jsonToForm( | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
| @@ -0,0 +1,16 @@ | |||
| import { type APIVariableScopeEnum, type VariableImportRequestVarsInner } from 'qovery-typescript-axios' | |||
|
|
|||
| export function formatData(data: { [key: string]: string }, keys: string[]) { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
| @@ -0,0 +1,15 @@ | |||
| export function onDrop(acceptedFiles: File[], handleData: (data: string) => void) { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
| @@ -0,0 +1,11 @@ | |||
| import { type UseFormSetValue } from 'react-hook-form' | |||
|
|
|||
| export function triggerToggleAll( | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
RemiBonnet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Summary
Added variables page at environment scope + various design improvments for all variables pages, namely:
Issue:
Screenshots / Recordings
Testing
yarn testoryarn test -u(if you need to regenerate snapshots)yarn formatyarn lintPR Checklist
.cursor/rules)feat(service): add new Terraform service) - required for semantic-release