Allow custom model for mobile devices (plus separate settings panel) #316
Conversation
This file contains 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
…op and mobile size
…is calculated because of an invalid custom model due to missing encoded values (recreation of CustomModelBox without the previously expected /info endpoint being called)
karussell
commented
Mar 16, 2023
| <div | ||
| ref={divElement} | ||
| className={styles.customModelBox} | ||
| style={{ display: enabled ? 'block' : 'none' }} |
There was a problem hiding this comment.
no need for this anymore as we keep the customModel in the settings and can handle the visibility of the customModelBox outside of this component
karussell
commented
Mar 16, 2023
src/stores/QueryStore.ts
Outdated
Comment on lines
106
to
110
| customModelEnabled: initialCustomModelStr != null, | ||
| customModelValid: false, | ||
| customModel: null, | ||
| zoom: true, | ||
| initialCustomModelStr: initialCustomModelStr, |
There was a problem hiding this comment.
moved them into the settings state
… avoid route re-calculation if settings box is shown; 3. invalid custom model is reset to custom model before if settings box is closed
… tests: 1. insist on transpiling custom model editor code to avoid ES6 modules for jest and 2. allowJs in tsconfig and 3. explicitly transform js files using ts-jest - why?
karussell
changed the title
This is a little bug we introduce when we added the custom model examples: We overwrite the current custom model with the default example, even when we entered our own model before. It can be fixed like this: |
…construction time)
* Move custom model examples to separate file * Minor rename * Extract custom model area function * Remove openSettingsHandle * Rename custom model examples file * Remove warning that can never occur * Move state from settings to query store, query store even depended on settings.. * minor * move custom model box out of settings * oops, made a mistake * Use string to represent custom model state! * much better * add todos * Rename action
karussell
changed the title
ZeroGxMax
pushed a commit
to minhhpkp/graphhopper-maps
that referenced
this pull request
Nov 12, 2024
…raphhopper#316) * initial version * custom model is kept even when switching back and forth between desktop and mobile size * fix bug when mobile view is changed to compact view and no new route is calculated because of an invalid custom model due to missing encoded values (recreation of CustomModelBox without the previously expected /info endpoint being called) * move CustomModelBox into SettingsBox but outside of settingsTable * minor -> warn * minor css fix * make settings css better fit the rest * 1. URL with custom_model should trigger a routing request with it; 2. avoid route re-calculation if settings box is shown; 3. invalid custom model is reset to custom model before if settings box is closed * tricky to fix error 'Cannot use import statement outside a module' in tests: 1. insist on transpiling custom model editor code to avoid ES6 modules for jest and 2. allowJs in tsconfig and 3. explicitly transform js files using ts-jest - why? * remove CustomModel from QueryStore state * formatting * revert change with SettingsContext to use ShowDistanceInMilesContext * format and clean up imports * show error message if invalid custom model (not possible at Settings construction time) * not many benefits to use validateJson as e.g. encoded values not yet known * revert commit 96108d7 * remove TODO NOW * Use string to represent custom model state (graphhopper#320) * Move custom model examples to separate file * Minor rename * Extract custom model area function * Remove openSettingsHandle * Rename custom model examples file * Remove warning that can never occur * Move state from settings to query store, query store even depended on settings.. * minor * move custom model box out of settings * oops, made a mistake * Use string to represent custom model state! * much better * add todos * Rename action * no shadow, use text instead * i18n: update * separate custom model box and settings box and use gear as indicator instead of shadow * format --------- Co-authored-by: easbar <[email protected]>
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.
In master the custom model is only active if the custom model box is visible which leads to bugs:
In this PR changes I basically moved the custom-model object state from the custom-model-editor into the SettingsStore that lives regardless of the UI component.
And so we can change how the gear wheel button works: the button now just opens a new settings box where we can change the miles/kilometer settings and also if the custom model is enabled or not. (A small shadow under the gear wheel is shown when the custom model is enabled. I'm open to suggestions to make it more obvious.)
And because the custom model editor does no longer store the custom model itself and also not if the custom model is enabled we don't have to always show the custom model editor and the above problems for small screen sizes disappear and we can additionally support custom models for mobile devices. Furthermore we can easily add new settings (like the dark mode option) without adding more stuff to the main view.
make json invalid -> hide settings box -> now default custom model example is shown instead of invalid (completely different) custom model before -> for now fixed it and closing the settings box means "reset to custom model before invalidated one". Is different than before but IMO makes sense now.You can try it out here.