Skip to content
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

Added fix for #9606 #9607

Merged
merged 2 commits into from
Nov 22, 2023
Merged

Conversation

diegovargasg
Copy link
Contributor

@diegovargasg diegovargasg commented Oct 16, 2023

Description

This code aims to solve the exception thrown at #9606

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x", remove the others)

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

Issue

What is the current behavior?
#9606

What is the new behavior?
The exception is solved and is posible to select a radius value with the arrow of the numeric input

Breaking change

  • Yes, and I documented them in migration notes
  • No

@tdipisa tdipisa added annotations related to the annotation tool bug labels Oct 16, 2023
@tdipisa tdipisa linked an issue Oct 16, 2023 that may be closed by this pull request
1 task
Copy link
Member

@tdipisa tdipisa left a comment

Choose a reason for hiding this comment

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

@diegovargasg thank you for contributing. Is it also possible for you to fix the unit tests that is now failing?

@diegovargasg
Copy link
Contributor Author

diegovargasg commented Oct 16, 2023

@diegovargasg thank you for contributing. Is it also possible for you to fix the unit test that is now failing?

yes, will do.

@offtherailz offtherailz changed the base branch from master to 2023.02.xx October 16, 2023 15:06
@offtherailz offtherailz changed the base branch from 2023.02.xx to master October 16, 2023 15:07
Copy link
Member

@tdipisa tdipisa left a comment

Choose a reason for hiding this comment

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

Thank you @diegovargasg. I've just realized in the issue #9606 that you are referring to the old annotation tool, the one currently on 2023.02.00, while in master we have an updated version of the tool that will land in 2024.01.00 and the issue is not present there. That would be a good fix for 2023.02.01 in any case. Let's wait for the review.

@tdipisa tdipisa requested review from allyoucanmap and removed request for offtherailz October 16, 2023 15:09
Copy link
Contributor

@allyoucanmap allyoucanmap left a comment

Choose a reason for hiding this comment

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

@diegovargasg thanks for the contribution! the code looks good to me and currently is not causing issue with the master because the new annotation panel is using only the first argument of onChangeRadius. @tdipisa we can merge this on master but the backport on 2023.02.xx is needed to actually fix the bug

@allyoucanmap allyoucanmap merged commit 3682efd into geosolutions-it:master Nov 22, 2023
6 checks passed
@allyoucanmap
Copy link
Contributor

@ElenaGallo this fix has no effect on master branch, I just prepared a backport for 2023.02.xx so we can test it directly there. Maybe we can make sure that the new annotation panel has not the same problem, thanks

@ElenaGallo ElenaGallo added the BackportNeeded Commits provided for an issue need to be backported to the milestone's stable branch label Nov 22, 2023
tdipisa pushed a commit that referenced this pull request Nov 22, 2023
@tdipisa tdipisa removed the BackportNeeded Commits provided for an issue need to be backported to the milestone's stable branch label Nov 22, 2023
tdipisa pushed a commit that referenced this pull request Nov 27, 2023
* Fix #9624 Point cloud shading options (#9666)

* Fix #9666 Include pointCloudShading option to saved layer config (#9670)

* #9606 Error with circle annotations + radius selection (#9607) (#9727)

Co-authored-by: Diego Vargas <[email protected]>

* Fix #9295 added better handling of format in csw service (#9712) (#9732)

* #9702: Fix - Background selector in contexts won't retain thumbnail in view mode (#9720) (#9744)

* #9567: handle functionality of zoom to record in table widgets (#9608)

* #9567: handle functionality of zoom to record in table widgets

* Fix: Correct failing test cases for zoom records issue in table widgets (#9567)
This commit addresses the failing test cases related to the issue of zoom records in table widgets.
* #9567: implement the new approach in zoom to records in table widgets + writing unit tests
* #9567: handle adding flag into config file to show/hide zoom icon for tblWidget
* #9567: reset flag enableZoomInTblWidget to be true for dashboard and map viewer
* #9567: resolve comments' review:
- put flag of zoomInTblWidget as a default prop
- add translations
- edit zoomToExtent enhancer to use internal zoom
- remove selector "getFlagOfShowingTblWidgetZoom " and use plugin prop instead

* #9683: add Details Panel for MS dashboard (#9689)

* #9683: add Details Panel for MS dashboard
-  The tool have the same options (eg. show as modal, show at startup etc.)
- The tool is defined in the same way of the corresponding one for maps.
- Edit the layout to put add widget & show/hide connection buttons to the sidebar menu

* #9683: resolve the FE test

Update DashboardEditor.jsx

* #9683: resolve review comments
* description:
- remove all dashboard selectors and pieces of code in generic components like sidebar plugin component that relevant to dashboard.
- add missing test for detailsLoaded action
- create new selectors, details uri selector and details settings se;ector that are used in many places in code
- move AddWidgetDashboard, MapConnedctionDashboard plugins to direct plugins folder
- Put global spinner in details plugin and remove it from sidebar plugin
-  edit in handleSave enhancer file to make update attributes of details just implemented for Map and Dashboard
- Add custom style in details.less as the lib of react-dock doesn't allow to override left css property
- remove unused added style from panels.less

* #9683: remove unused comments in dashboard-test file

* #9683: edit in details epics and selectors to fix FE test

* #9683: Resolve review comments
Description:
- Reolve unused loading property from DashoardEditor file
- Add tooltip for save dashboard
- Remove custom style in BorderLayout and leave it with generic style

* #9683: resolve review comments
Description:
- edit navbar.less files to fix going language selector behind body panel
- remove unused z-index in dashboard.less
# Conflicts:
#	web/client/epics/__tests__/config-test.js
#	web/client/epics/config.js

* #9683: resolve test comment (#9730)

- Adding export, import, delete dashboard
- Reorder shown plugins in sidebar for dashboard

* #9683: add Details Panel for MS dashboard [Editing the detail panel tooltip and title]  (#9740)

* #9683: resolve test comment
Description:
- edit the detail panel tooltip and shown title and make it generic one
- Add translations for the new tooltip

* Update web/client/translations/data.it-IT.json

---------

Co-authored-by: Matteo V <[email protected]>

* #9728 fix misalignement issue (#9731) (#9742)

* Fix #9729 fixed formats in catalog used in dashboard (#9733) (#9747)

---------

Co-authored-by: stefano bovio <[email protected]>
Co-authored-by: Diego Vargas <[email protected]>
Co-authored-by: Suren <[email protected]>
Co-authored-by: mahmoud adel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
annotations related to the annotation tool bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error with circle annotations + radius selection
4 participants