-
Notifications
You must be signed in to change notification settings - Fork 3
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
improve gui for pipeline job creation #1141
Conversation
nilsgstrabo
commented
Nov 22, 2024
•
edited
Loading
edited
- Make links in environment card wrap if long text
- Show info alert on pipeline job and environments summary page when no environments (no radixconfig)
- Default to apply-config on "create new" pipeline job when no environments (no radixconfig)
- Misc styling fixes
…ation about what to do with it
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.
La inn ett par kommentarer, useMemo og useCallback tror jeg ikke er nødvendig?
(fun fact, React 19 vil komme med en compiler som automatisk legger inn useMemo/useCallback overalt, så vi trenger ikke håndtere det manuelt)
!nonErrorCodes?.includes(getFetchErrorCode(asyncState.error)) | ||
asyncState.isError || | ||
(asyncState.error && | ||
!nonErrorCodes?.includes(getFetchErrorCode(asyncState.error))) |
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.
Vil dette fungera? asyncState.isError vil alltid ver sann når asyncState.error e satt, så nonErrorCodes vil vel aldri ha en effekt lengre? 🤔
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.
Fixed
const setPipelineType = useCallback( | ||
(pipeline: string) => { | ||
setSearchParams((prev) => { | ||
prev.set(pipelineSearchParam, pipeline); | ||
return prev; | ||
}); | ||
}, | ||
[setSearchParams] | ||
); |
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.
e det nødvendig med useCallback?
const setPipelineType = useCallback( | |
(pipeline: string) => { | |
setSearchParams((prev) => { | |
prev.set(pipelineSearchParam, pipeline); | |
return prev; | |
}); | |
}, | |
[setSearchParams] | |
); | |
const setPipelineType = (pipeline: string) => { | |
setSearchParams((prev) => { | |
prev.set(pipelineSearchParam, pipeline); | |
return prev; | |
}); | |
}; |
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.
fixed
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!