search

Helium314 / SCEE

OpenStreetMap surveyor app for experienced OSM contributors
GNU General Public License v3.0
116 stars 8 forks source link

#506 Add background type Custom #564

Open ravenfeld opened 1 week ago

ravenfeld commented 1 week ago

I take the liberty of reading a little of the issues and this one was within my technical reach.

I didn't simply replace the value when there is data in the url but added a new entry. I thought that maybe some people still wanted to keep Esri and use a custom background at the same time.

The only problem is that I find there's a lot of duplicate code for the toogles. Is a refactor acceptable or do you prefer it this way?

Screenshot_20240703_154554

mnalis commented 1 week ago

I see the use of such functionality; however, how does quick-button Toggle aerial / map background works then?

Does it always cycle through all 3 of them? Or does it skip custom one if it is empty?

ravenfeld commented 1 week ago

At the moment, it goes through all 3 states because I haven't redefined the default value for custom. Custom by default is on ESRI, maybe I should set it to empty and propose that custom is filled in by the user?

mnalis commented 1 week ago

Custom by default is on ESRI, maybe I should set it to empty and propose that custom is filled in by the user?

That sound good to me, if the result is that if Custom is empty, in only switches between 2 values (OSM/ESRI)! (instead of e.g. SCEE still trying to cycle through all 3 values, but displaying empty screen or crashing when reaching empty Custom - which would be bad result, obviously)

ravenfeld commented 1 week ago

Currently there is a check in the modification of the tile url. It cannot be empty.

So on a first installation : MAP ESRI CUSTOM=ESRI url

Which could be weird, maybe using OSM tiles?

Helium314 commented 1 day ago

I'm fine with the duplicate code (also: it was already duplicate, so I'm not really in a position to complain). Though putting it in cycleBackground(prefs) and getBackgroundStringId(background) functions should not be much work.

Though I would like to avoid changes to existing behavior when the user didn't set a custom tile source. Maybe make it empty by default, allow empty URL, and skip custom from toggle / cycling background if empty.