Skip to content

Vue migration to composition api - #2464

Open
linusbbb wants to merge 42 commits into
eclipse-set:mainfrom
linusbbb:vue-migration-to-composition-api
Open

linusbbb wants to merge 42 commits into
eclipse-set:mainfrom
linusbbb:vue-migration-to-composition-api

Conversation

@linusbbb

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread web/siteplan/src/components/development/FeatureSearch.vue Outdated
Comment thread web/siteplan/src/components/development/FeatureSearch.vue Outdated
Comment thread web/siteplan/src/components/development/FeatureSearch.vue Outdated
@linusbbb
linusbbb requested a review from mariusheine June 4, 2026 05:57
Comment thread web/siteplan/src/components/development/LayerControl.vue Outdated
Comment thread web/siteplan/src/components/development/ModelSummaryControl.vue
Comment thread web/siteplan/src/components/popup/LockKeyPopup.vue Outdated
Comment thread web/siteplan/src/components/popup/MenuPopup.vue
Comment thread web/siteplan/src/components/popup/MenuPopup.vue Outdated
Comment thread web/siteplan/src/components/toolbar/KatalogMenuBar.vue Outdated
Comment thread web/siteplan/src/components/toolbar/Toolbar.vue Outdated
Comment thread web/siteplan/src/components/toolbar/Toolbar.vue Outdated
Comment thread web/siteplan/src/components/MapContainer.vue Outdated
Comment thread web/siteplan/src/components/MapContainer.vue
Comment thread web/siteplan/src/components/popup/LockKeyPopup.vue Outdated
Comment thread web/siteplan/src/components/popup/MenuPopup.vue Outdated
Comment on lines +87 to +91
watch(selectedFeature, value => {
if (value && getFeatureType(value) === FeatureType.Flash) {
selectedFeature.value = (getFeatureData(value) as FlashFeatureData).refFeature
}
}, { flush: 'sync' })

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@linusbbb In my opinion we should not use a watcher on a variable and the watcher is changing this variable again. This could potentially lead to infinite loops in the future.

The better option would be to do this check inside the function selectedItem.


watch(features, () => {
if (features.value.length === 1) {
selectedFeature.value = features.value[0]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@linusbbb

Suggested change
selectedFeature.value = features.value[0]
selectedItem(features.value[0])

if (features.value.length === 1) {
selectedFeature.value = features.value[0]
}
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@linusbbb If you mount the component immediately with only one feature this watcher would not have been executed. That's why we need

Suggested change
})
}, { immediate: true })

selectedFeature.value = null
}

function selectedItem (feature: Feature<Geometry>) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@linusbbb I would recommend to rename this function to

Suggested change
function selectedItem (feature: Feature<Geometry>) {
function selectFeature (feature: Feature<Geometry>) {

this.mastList = this.svgService.getSvgElementInGroup(SVGMast.ShortMount)
}
function getZusatzSignal (): ZusatzSignal[] {
return (svgService.getSvgElementInGroup(AndereSignalGroup.ZusatzSignale) ?? []) as ZusatzSignal[]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@linusbbb Why do we need the type cast here? We also don't have it in getShortMast

Suggested change
return (svgService.getSvgElementInGroup(AndereSignalGroup.ZusatzSignale) ?? []) as ZusatzSignal[]
return svgService.getSvgElementInGroup(AndereSignalGroup.ZusatzSignale) ?? []

const compareProps = [
{
prop: getpropertypeName(defaultStationObj(), x => x.label),
prop: getpropertypeName(defaultStationObj(), x => x.label!),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@linusbbb Is this really safe? In my opinion we should use a fallback e.g. like

Suggested change
prop: getpropertypeName(defaultStationObj(), x => x.label!),
prop: getpropertypeName(defaultStationObj(), x => x.label ?? ''),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@linusbbb Here we should fix the length of the "Export PPM" input element

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@linusbbb Here we should also fix the length of the select element

@mariusheine

Copy link
Copy Markdown
Contributor

@linusbbb And please resolve the merge conflicts and the failing typecheck

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants