Fix list ordering in import/export and incomplete Select all in Tracks - #5763
Merged
Merged
Conversation
The types inside a category were taken by enumerating an NSDictionary, so they came out in hash order and the list was arranged differently on every run. Walk OAExportSettingsType.getAllValues instead, which is the order the rest of the export code uses and the one Android lists them in. Category order was already fixed; only the types within a category moved.
Tracks sitting directly in the current folder were left out of the selection whenever their file name happened to contain a subfolder's name: the filter tested track.gpxFilePath.contains(folderName) against the whole path, file name included. With folders named "color" or "test" that hits a lot of them, so deleting had to be done in two passes. Compare whole path components instead, and share the check with areAllItemsSelected(), which already did it properly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two independent list bugs in the same area.
1. Import and export type lists come out in a different order every run
On the import selection screen (and the export screen) the data types inside a category are arranged differently each time:
Tracks, Navigation history, Search history, Favourites, OSM Edits, Markers history, Map markersone run, something else the next.+[OASettingsHelper getSettingsToOperateByCategory:addEmptyItems:]fills the per-categoryMutableOrderedDictionaryby enumeratingsettingsToOperate, which is a plainNSDictionary— so insertion happens in hash order, andOASettingsCategoryItems.getTypes(allKeysof the ordered dictionary) hands that order straight to the table.Fix: walk
OAExportSettingsType.getAllValuesand pick the types that are present. That is the canonical order the rest of the export/import code already uses, and it matches Android, whereSettingsHelper.collectExportDataiteratesExportType.availableValuesOf(category)into aLinkedHashMap.My Places now reads:
Notes:
getSettingsToOperate:can produce are present ingetAllValues, so nothing is dropped.MutableOrderedDictionary.allKeysreturns[_keys array], i.e. insertion order, so fixing insertion is enough.2. "Select all" in Tracks does not select everything
Selecting all in a track folder leaves tracks unselected, so deleting or sharing has to be repeated. Observed on a folder of 782 tracks:
Select allcovered 653 of them.onSelectDeselectAllButtonClickedselects every subfolder, then adds the tracks of the current folder minus the ones considered to live inside a selected subfolder:That is a substring test over the whole path, file name included. A root track called
test1.gpxmatches the foldertest; anything withcolorin its name matches the foldercolor. Those tracks are silently dropped from the selection. The exclusion is not even needed in principle —TrackFolder.getTrackItems()is not recursive, so a direct child can never be inside a subfolder.Fix: compare whole path components, and share one
isTrack(_:insideSelectedFolderOf:)helper withareAllItemsSelected(), which already did the path comparison properly. The helper also requires a separator after the prefix, so foldertestno longer matches a track sitting intest2.