-
Notifications
You must be signed in to change notification settings - Fork 7
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
Unified pipeline studio view component #1161
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for harness-xd-review ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for harness-design ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
is this the best place to keep these re-usable components?
why not in a separate package, or maybe in the unified-pipeline
package?
import { TextAreaInput } from '../TextAreaInput' | ||
import { TextInput } from '../TextInput' | ||
|
||
const inputComponentFactory = new InputFactory() |
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.
i think this factory should be instantiated in gitness
or harness
apps separately.
I imagine we would be registering different components in both.
few differences I can think of:
- Connectors in Platform, but not present in Opensource
- Runtime inputs/expressions can be different in both
|
||
SectionItem: function SectionHeader({ children }: { children: React.ReactNode }) { | ||
return ( | ||
<div className="hover:bg-shade-10 mb-3 flex flex-col rounded-md border bg-gradient-to-br from-[#FFFFFF15] to-[#FFFFFF00] p-2"> |
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.
hardcoded hex codes should be avoided
@@ -0,0 +1,15 @@ | |||
const StepsPaletteLayout = { |
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.
I doubt if we need so much componentisation for every single div.
this will not be reused beyond 1 or (maybe) 2 places.
</StepsPaletteContentLayout.SectionItem> | ||
)) | ||
) : ( | ||
<p className="text-muted-foreground">There is no steps for provided query.</p> |
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.
i18n?
export function UnifiedPipelineStudioProblemsPanel({ problems }: { problems: any }): JSX.Element { | ||
const { updateCursorPosition } = useYamlEditorContext() | ||
|
||
return problems.length === 0 ? ( |
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.
do we need this wrapper just for this condition?
can we not just return NoProblemsFound
from <Problems />
if the passed array is empty?
No description provided.