Skip to content

PB-540: Adapt help text for drawing#1296

Merged
ismailsunni merged 4 commits intodevelopfrom
feat-pb-540-drawing-tool-adapt-help-text
Apr 17, 2025
Merged

PB-540: Adapt help text for drawing#1296
ismailsunni merged 4 commits intodevelopfrom
feat-pb-540-drawing-tool-adapt-help-text

Conversation

@ismailsunni
Copy link
Contributor

@ismailsunni ismailsunni commented Apr 14, 2025

@cypress
Copy link

cypress bot commented Apr 14, 2025

web-mapviewer    Run #5062

Run Properties:  status check passed Passed #5062  •  git commit 8740a884f7: Merge pull request #1296 from geoadmin/feat-pb-540-drawing-tool-adapt-help-text
Project web-mapviewer
Branch Review develop
Run status status check passed Passed #5062
Run duration 01m 23s
Commit git commit 8740a884f7: Merge pull request #1296 from geoadmin/feat-pb-540-drawing-tool-adapt-help-text
Committer Ismail Sunni
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 1
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 48
View all changes introduced in this branch ↗︎

@ismailsunni ismailsunni force-pushed the feat-pb-540-drawing-tool-adapt-help-text branch from eff7c6a to de29456 Compare April 16, 2025 07:24
@ismailsunni ismailsunni requested a review from Copilot April 16, 2025 07:24
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 7 changed files in this pull request and generated no comments.

Files not reviewed (5)
  • packages/mapviewer/src/modules/i18n/locales/de.json: Language not supported
  • packages/mapviewer/src/modules/i18n/locales/en.json: Language not supported
  • packages/mapviewer/src/modules/i18n/locales/fr.json: Language not supported
  • packages/mapviewer/src/modules/i18n/locales/it.json: Language not supported
  • packages/mapviewer/src/modules/i18n/locales/rm.json: Language not supported
Comments suppressed due to low confidence (1)

packages/mapviewer/src/modules/drawing/components/DrawingTooltip.vue:131

  • Ensure that 'selectedFeatures' is non-empty before accessing 'selectedFeatures.value[0]' to avoid potential runtime errors, for example by inserting a check prior to using it.
if (nonPointFeatureTypes.includes(featureDrawingMode)) {

@ismailsunni ismailsunni force-pushed the feat-pb-540-drawing-tool-adapt-help-text branch from de29456 to 24dbe9e Compare April 16, 2025 12:56
@ismailsunni ismailsunni requested a review from Copilot April 16, 2025 13:37
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adapts the help text for drawing features by adjusting tooltip translation keys and revising deletion logic in the drawing toolbox.

  • In DrawingTooltip.vue, a new array for non-point feature types is added and logic is introduced to append the geometry type to the translation keys when applicable.
  • In DrawingToolbox.vue, the deletion logic is updated by renaming and reworking the computed property to more clearly allow deletion of the last point based on drawing mode.

Reviewed Changes

Copilot reviewed 2 out of 7 changed files in this pull request and generated no comments.

File Description
packages/mapviewer/src/modules/drawing/components/DrawingTooltip.vue Adds non-point feature types and refines tooltip translation key logic for vertex modification.
packages/mapviewer/src/modules/drawing/components/DrawingToolbox.vue Renames and updates the computed property controlling point deletion to support extended line drawing scenarios.
Files not reviewed (5)
  • packages/mapviewer/src/modules/i18n/locales/de.json: Language not supported
  • packages/mapviewer/src/modules/i18n/locales/en.json: Language not supported
  • packages/mapviewer/src/modules/i18n/locales/fr.json: Language not supported
  • packages/mapviewer/src/modules/i18n/locales/it.json: Language not supported
  • packages/mapviewer/src/modules/i18n/locales/rm.json: Language not supported

@ismailsunni ismailsunni requested review from ltkum, pakb and sommerfe April 16, 2025 13:38
Copy link
Contributor

@sommerfe sommerfe left a comment

Choose a reason for hiding this comment

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

can be maybe simplified but it's ok for readability

Comment on lines +67 to +80
const isAllowDeleteLastPoint = computed(
() => {
// Allow to delete the last point only if we are drawing line or measure
if (isDrawingLineOrMeasure.value) {
return true
} else {
// or when extending line
return (
editMode.value === EditMode.EXTEND &&
selectedLineString.value &&
selectedLineCoordinates.value?.length > 2
)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const isAllowDeleteLastPoint = computed(
() => {
// Allow to delete the last point only if we are drawing line or measure
if (isDrawingLineOrMeasure.value) {
return true
} else {
// or when extending line
return (
editMode.value === EditMode.EXTEND &&
selectedLineString.value &&
selectedLineCoordinates.value?.length > 2
)
}
}
const isAllowDeleteLastPoint = computed(
() => {
// Allow to delete the last point only if we are drawing line or measure
return isDrawingLineOrMeasure.value) ||
// or when extending line
(
editMode.value === EditMode.EXTEND &&
selectedLineString.value &&
selectedLineCoordinates.value?.length > 2
)
}
}

@ismailsunni ismailsunni force-pushed the feat-pb-540-drawing-tool-adapt-help-text branch from 24dbe9e to 0b2a235 Compare April 17, 2025 04:50
@ismailsunni ismailsunni merged commit 8740a88 into develop Apr 17, 2025
6 checks passed
@ismailsunni ismailsunni deleted the feat-pb-540-drawing-tool-adapt-help-text branch April 17, 2025 05:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants