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

Fix/3478/reset camera global option broken #3699

Merged
merged 14 commits into from
Aug 30, 2024

Conversation

IdrisLaabidi
Copy link
Contributor

@IdrisLaabidi IdrisLaabidi commented Aug 23, 2024

Fix camera breaking because of the global option 'reset camera when changing map'

Issue: #3478

Description

Definition of Done

A PR is only ready for merge once all the following acceptance criteria are fulfilled:

  • Changes have been manually tested
  • All TODOs related to this PR have been closed
  • There are automated tests for newly written code and bug fixes
  • All bugs discovered while working on this PR have been submitted as issues (if not already an open issue)
  • Documentation (GH-pages, analysis/visualization READMEs, parser READMEs, --help, etc.) has been updated (almost always necessary except for bug fixes)
  • CHANGELOG.md has been updated

Screenshots or gifs

@IdrisLaabidi IdrisLaabidi force-pushed the fix/3478/reset-camera-global-option-broken branch from 829aa7e to b0229c6 Compare August 23, 2024 12:02
Copy link
Collaborator

@IhsenBouallegue IhsenBouallegue left a comment

Choose a reason for hiding this comment

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

Just minor nitpicks otherwise looks good 👍

@@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/)
### Fixed 🐞

- Fix issue where zooming out too much makes the map disappear and zooming in too much causes you to go through the map. [#3697](https://github.com/MaibornWolff/codecharta/pull/3697)
- Camera perspective is correctly adopted from the custom configuration[#3194](https://github.com/MaibornWolff/codecharta/pull/3194)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not so sure why, but this redirects to an issue. According to our guidelines it should be a PR, preferably this one.

@@ -29,7 +31,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/)
### Fixed 🐞

- Revert [#3655](https://github.com/MaibornWolff/codecharta/pull/3665) as we implement new navigation methods
- Camera perspective is correctly adopted from the custom configuration[#3194](https://github.com/MaibornWolff/codecharta/pull/3194)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need to change the changelog of previous versions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, that's a recent bug fix, it has nothing to do with the previous version. It was there by mistake, and I corrected it

await wait(0)

expect(threeCameraService.camera.position).toEqual(new Vector3(0, 0, 0))
//expect(threeRendererService.render).not.toHaveBeenCalled()
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove commented line if not being used

Copy link

sonarcloud bot commented Aug 30, 2024

Copy link

sonarcloud bot commented Aug 30, 2024

@IdrisLaabidi IdrisLaabidi merged commit 519e05d into main Aug 30, 2024
7 checks passed
@IdrisLaabidi IdrisLaabidi deleted the fix/3478/reset-camera-global-option-broken branch August 30, 2024 08:44
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