PB-375: Added drawing to the report a problem #805
Merged
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
ltshb
changed the title
Passing run #1958 ↗︎
Details:
Review all test suite changes for PR #805 ↗︎ |
|||||||||||||||
ltshb
force-pushed
the
feat-PB-375-drawing-report
branch
5 times, most recently
from
693a148 to
18c8477
Compare
ltshb
force-pushed
the
feat-PB-375-drawing-report
branch
from
42e0253 to
4379851
Compare
pakb
reviewed
Apr 25, 2024
hansmannj
reviewed
Apr 25, 2024
There was a problem hiding this comment.
Thanks, did not check the code, but only the test link.
Two minor things:
- when clicking on "Draw on the map" as a first action after clicking on "report a problem", i.e. the text field is still empty, sometimes the app does not switch into the drawing mode but is stuck and complains "Report message can not be empty.". Sometimes it switches into the drawing mode, but the red focus and message still shortly appears.
- Perhaps the text "indicate the appropriate location on the map" is not optimal, as it is not always about a certain location. Maybe something like "Please create the drawing you want to attach" or similar might be better. Picky, I know, sorry! 😉
@hansmannj for the first point regarding field validation I propose to do it in a separate PR as we should change the field validation behaviour globally not only here. The issue is that we validate the field on typing and focus out. |
pakb
approved these changes
Apr 29, 2024
Comment on lines
+36
to
+38
| return !!store.state.layers.systemLayers.find( | ||
| (l) => l.id === store.state.drawing.temporaryKmlId | ||
| ) |
There was a problem hiding this comment.
could be switched to return store.state.layers.systemlayers.some(...) if the goal is to have a boolean return value
hansmannj
approved these changes
Apr 29, 2024
This is needed for the future report a problem that will open the drawing mode and were we don't want to loose the form values (the form is in the menu header).
For this I made the drawing online configurable, which means that the drawing is either online saved (kml backend) and added to the active layers, or local and added to the system layers.
ltshb
force-pushed
the
feat-PB-375-drawing-report
branch
from
4c5a50f to
7241d4d
Compare
Avoid overflow in the drawing title which would make the text outside of the header. Currently the header height is fixed which has some issues in the placement of elements.
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.
Note with normal size it works good and on mobile the title is anyway not displayed.
Test link