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 when changing between 2 custom overlays #566

Open
wants to merge 2 commits into
base: modified
Choose a base branch
from

Conversation

ravenfeld
Copy link

@ravenfeld ravenfeld commented Jul 2, 2024

I noticed that when I changed the overlay between 2 customs the icon didn't update.
There was even the beginnings of a fix in the code, but that doesn't change anything because it wasn't a view refresh problem.

This is due to the fact that the reference was not changed because there is only one instance in the OverlayRegistry.

  • So I've changed it to use a reference on the objects.
  • I've also removed the need to always look at the prefs (pref level is not recommended by google and it's not even asynchronous).

I hope it won't be a problem during the rebases with SC

@Helium314
Copy link
Owner

I would rather have minor issues in SCEE than larger deviation from SC, because SC code changes frequently and merging can be quite time consuming.

So if possible, try to keep code that is the same as SC stable, and do the changes is SCEE-only code sections. (I did not check, because SC merge created merge conflicts already)

Btw I guess this change will be gone anyway when the involved parts are changed to compose.

@ravenfeld
Copy link
Author

I'll see if it's still there with the new SC code.

@Helium314
Copy link
Owner

I had another short look, and I think it would be best to wait until main screen UI is re-done in compose.
Not sure how long it will take... currently the idea is to do it right after switching to MapLibre, but that's blocked by a rather critical bug.

@ravenfeld
Copy link
Author

Don't worry, you want me to create a label for the PR we're blocking? Like an "on hold" label?

@Helium314 Helium314 added the blocked waiting for changes somewhere else label Jul 28, 2024
@Helium314
Copy link
Owner

Good idea, I added a label.

@Helium314 Helium314 linked an issue Sep 28, 2024 that may be closed by this pull request
@ravenfeld
Copy link
Author

@Helium314 Hi, I'm back from holiday and back to work.
I was wondering if the compose version of SC is more stable now and if I could get back to work on this branch?

@Helium314
Copy link
Owner

Sorry for the late reply. I would like to wait until streetcomplete#5799 is implemented.

@Helium314 Helium314 removed the blocked waiting for changes somewhere else label Dec 27, 2024
@Helium314
Copy link
Owner

The main UI is now in compose, including the overlay dropdown. Looks like icon now changes when switching between custom overlays.
I only coarsely looked over the PR now, and it seems you also have changes that don't seem clearly related to the icon. Is there anything left you think should be implemented?

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.

Switching custom overlays don't update the rendering
2 participants