Sofa 310 gantt chart clean structure #180
Conversation
…-gantt-chart-component # Conflicts: # vue/package-lock.json # vue/package.json
…nd week view for mini stacking
…-chart-clean-structure # Conflicts: # vue/package-lock.json # vue/package.json
| const keys = path.split('.') | ||
| const { parent, lastKey } = traversePath(result, keys) | ||
|
|
||
| /* eslint-disable security/detect-object-injection */ |
| const props = withDefaults(defineProps<GanttChartComponentProps>(), { | ||
| dateRange: () => [], | ||
| rows: () => [], | ||
| links: () => [], | ||
| headerLabel: 'gantt_chart.header', | ||
| viewMode: 'day', | ||
| showWeekendShading: true, | ||
| stackMiniActivities: true, | ||
| activityTooltip: undefined, | ||
| activityClick: undefined, | ||
| leftHeaderWidthPx: 320, | ||
| }) |
There was a problem hiding this comment.
I don't know why LLM's are stuck with this old definition of props with default values 😆
The new way of doing that is like this:
const { viewMode = 'day', ... } = defineProps<GanttChartComponentProps>()
Beware that use of these props in the code will have to remove the props prefix
| emit('activityClick', activity, rowData) | ||
| } | ||
| const viewMode = computed(() => props.viewMode ?? 'day') |
There was a problem hiding this comment.
This specific computed value should then disappear as a default viewMode will be destructured from the above props definition :)
| } | ||
| const props = withDefaults(defineProps<GanttChartComponentProps>(), { | ||
| dateRange: () => [], |
There was a problem hiding this comment.
Should this be directly the array of dates?
I would more expect the API of this Gantt to ask for a startDate and endDate and then compute this range.
I am not 100% sure at this stage of my review but as many other thing place themselves based on an offset starting from the start_date, if you give a weird daterange with missing values, the elements like activities could position themselves and rekt the layout 😄
| <GanttChartRowsList | ||
| :list="list" | ||
| :row-heights="rowHeights" | ||
| :date-range="props.dateRange" | ||
| :view-mode="viewMode" | ||
| :show-weekend-shading="props.showWeekendShading" | ||
| :stack-mini-activities="props.stackMiniActivities" | ||
| :activity-tooltip="props.activityTooltip" | ||
| :left-header-width-px="props.leftHeaderWidthPx" | ||
| mode="labels" |
There was a problem hiding this comment.
I was at first ok with this but now I analyze this closer, many props does not apply at all to this label list like:
activity-tooltipstack-mini-activitiesshow-weekend-shading
Maybe we should consider split in distinct sub components instead of using the same one
| <GanttChartHeader | ||
| :date-range="props.dateRange" | ||
| :view-mode="viewMode" | ||
| :header-label="props.headerLabel" | ||
| :left-header-width-px="props.leftHeaderWidthPx" | ||
| :show-grid="false" | ||
| /> | ||
| </div> | ||
| </template> | ||
| <template #header-right> | ||
| <div ref="gantt-header" class="w-full"> | ||
| <GanttChartHeader | ||
| :date-range="props.dateRange" | ||
| :view-mode="viewMode" | ||
| :header-label="props.headerLabel" | ||
| :left-header-width-px="props.leftHeaderWidthPx" | ||
| :show-left-header="false" | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
Same here with reuse, the header-left is really a dummy spacing element to align properly the dates indicators of the header-right yet they reuse the same component with a large chunk of things header-left probably does not care.
I would advocate to split
| @@ -0,0 +1,62 @@ | |||
| <template> | |||
There was a problem hiding this comment.
Maybe add comments to explain the purpose of this component
| v-if="props.showLeftHeader" | ||
| class="left-0 flex shrink-0 sticky z-60 justify-center items-center box-border border-r border-b border-surface-200 bg-surface-50 text-surface-900 font-semibold h-full" | ||
| :style="{ width: `${props.leftHeaderWidthPx}px` }" | ||
| > | ||
| {{ t(props.headerLabel) }} | ||
| </div> | ||
| <!-- Headers --> | ||
| <div v-if="props.showGrid" class="flex flex-col"> |
There was a problem hiding this comment.
As said elsewhere, from what I get is those 2 v-if are in fact exclusives?
And thus a split of component would make sense
| @@ -0,0 +1,98 @@ | |||
| <template> | |||
There was a problem hiding this comment.
Maybe a small explanation over the SVG creation logic here would be cool 😄
No description provided.