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

assign null value in merge util. fire changeend event on start. #967

Merged
merged 6 commits into from
Oct 17, 2023

Conversation

dbauszus-glx
Copy link
Member

The merge util incorrectly assigned an empty object instead of null.

The changeEnd event must be fired when the mapview is initialised. Otherwise the event would not be fired with no layer on display. The table check would be omitted in that case.

@dbauszus-glx dbauszus-glx added the Bug A genuine bug. There must be some form of error exception to work with. label Oct 17, 2023
lib/utils/merge.mjs Dismissed Show resolved Hide resolved
@RobAndrewHurst
Copy link
Contributor

RobAndrewHurst commented Oct 17, 2023

image

I have noticed something where if you turn a layer on and then zoom past the allowed min zoom the layer will still render.

Would the desired behaviour be that the canvas is cleared of that layer?

@dbauszus-glx
Copy link
Member Author

@RobAndrewHurst That shouldn't happen. I will have a look to address the issue.

@dbauszus-glx
Copy link
Member Author

@RobAndrewHurst should be fixed in last commit.

@RobAndrewHurst
Copy link
Contributor

Nice one! Thanks @dbauszus-glx

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Contributor

@RobAndrewHurst RobAndrewHurst left a comment

Choose a reason for hiding this comment

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

Looks all good to me

Copy link
Contributor

@simon-leech simon-leech left a comment

Choose a reason for hiding this comment

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

This is working for me and fixes the described bug. Happy to merge!

@dbauszus-glx dbauszus-glx merged commit 26935e0 into GEOLYTIX:main Oct 17, 2023
6 checks passed
@dbauszus-glx dbauszus-glx deleted the null-merge-changeend branch January 29, 2024 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A genuine bug. There must be some form of error exception to work with.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants