initial changes for Gantt-like feature #430
Conversation
…imeline component can both toggle it
| const { timelineViewActive, disableTimelineView } = useTimelineViewStore((state) => ({ | ||
| timelineViewActive: state.timelineViewActive, | ||
| disableTimelineView: state.disableTimelineView, | ||
| })); |
There was a problem hiding this comment.
To use multiple props from a zustand store, you should just have one line for each. Otherwise you re-create the selector output on every state change, even unrelated ones.
| const childrenArray = Children.toArray(children); | ||
|
|
There was a problem hiding this comment.
If you expect certain children to be present in a certain order, you should just use named props. children is supposed to be a bit like a black box and there is no real way to limit the type.
| // always show bpmn-js modeler when opening a process | ||
| useEffect(() => { | ||
| disableTimelineView(); | ||
| }, [processId]); |
There was a problem hiding this comment.
You should probably reset the store when the timeline component unmounts instead of this.
| @@ -45,7 +49,14 @@ const Process = async ({ params: { processId, environmentId }, searchParams }: P | |||
| process={{ ...process, bpmn: selectedVersionBpmn as string }} | |||
| versions={process.versions} | |||
| versionName={selectedVersion?.name} | |||
| timelineViewFeatureEnabled={timelineViewFeatureEnabled} // required for .env feature flag | |||
There was a problem hiding this comment.
Just as a note: you can use env vars in client components by prefixing them with NEXT_PUBLIC_ , but we can leave this since we might combine it with DB config values.
| import { useEnvironment } from './auth-can'; | ||
| import { Element, Parent } from 'diagram-js/lib/model/Types'; | ||
| import { useEffect, useRef, useState } from 'react'; | ||
| import useTimelineViewStore from '@/app/(dashboard)/[environmentId]/processes/[processId]/use-timeline-view-store'; |
There was a problem hiding this comment.
You can move the store into the lib folder, if it is being used in multiple locations. This static import path to the page folder is more at risk of change in the future.
|
@OhKai thank you for the feedback. I've implemented the changes suggested by you and opted to change the env variable as well, as in its current usage it improves clarity in my opinion. |
|
@luckyTamme Can you try to resolve the merge conflicts, so I can merge this PR? If you run into problems, I can help you with that. |
|
@OhKai I'll do some changes that I've talked with Kai about and will resolve the conflicts end of this week / next week. |
|
@OhKai the changes are implemented and all merge conflicts resolved. excuse my messy commit history, I had some issues with my local setup 😅 |
|
Hi @OhKai, can we merge my changes or are changes required? |
For my bachelor thesis, I will implement a Gantt-like timeline view for processes that should work synchronously with bpmn diagram modulation. to keep the impact on the main project as small as possible and not to obstruct any ongoing developments, most work will happen in the new bpmn-timeline.tsx component.
In order to render either the bpmn-timeline or the existing bpmn-modeler, some adjustments to the process page.tsx and wrapper had to be made.
Because the toggle for switching to the timeline-mode should be inside the toolbar, a new state-store had to be introduced. There might be a more elegant way of realising this and with less overhead, but this was the best way I could implement it due to the page.tsx being a server component and how the client component structure with children inside wrapper.tsx works.