PB-66: Add new babs icons to mapviewer#1058
Conversation
web-mapviewer
|
||||||||||||||||||||||||||||
| Project |
web-mapviewer
|
| Branch Review |
feat-PB-66-add-new-babs-icons-to-mapviewer
|
| Run status |
|
| Run duration | 05m 03s |
| Commit |
|
| Committer | Lukas Joss |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
21
|
Skipped
|
0
|
Passing
|
210
|
| View all changes introduced in this branch ↗︎ | |
27b5892 to
3b81fc4
Compare
2639661 to
0d6c2b8
Compare
cbfede4 to
e8c864e
Compare
pakb
left a comment
There was a problem hiding this comment.
couple questions about the whole thing :
- why have this description URL? We won't be using it, as i18n keys are then also given in the list of icons endpoint.
- I'm wondering if we should "tag" the sets with a
langISO code value, so that we may have a lang selector alongside the choice of "Civil icons", instead of having 3 times the civil icons in the list - When showing description on icons that aren't available in the current app lang, I would show them all (prefixed with ISO code)
- I would maybe add the description of the icon to the description of the marker, would make sense to know what kind of icon it is also outside of the drawing module
Should the backend for the set instead have a boolean, e.g. has_description?
I assume this should also be provided by the back-end?
What do you mean with "icons that aren't available in the current app lang"? All icons should be available in all languages
|
ltkum
left a comment
There was a problem hiding this comment.
Solid work. I'm not sure about the need for the translation parameter in this case (but I did not work with the icons, so maybe some descriptions will have problematic 'keys'), and I would alter the switch a bit, but I think this is otherwise good.
| translate | ||
| ? i18n.t(tp.reference.attributes['data-tippy-content'].value) | ||
| : tp.reference.attributes['data-tippy-content'].value | ||
| ) |
There was a problem hiding this comment.
When you give a key that doesn't exist, i18n will return the key itself and won't translate anything. Unless there is a use case I don't see, I think you don't need to add this 'translate' parameter.
There was a problem hiding this comment.
If I don't set it to false it will search through the entire dictionary for every icon which makes it laggy
8f337df to
23f9e0b
Compare
edc0082 to
30cc524
Compare
src/modules/infobox/components/styling/DrawingStyleIconSelector.vue
Outdated
Show resolved
Hide resolved
src/modules/infobox/components/styling/DrawingStyleIconSelector.vue
Outdated
Show resolved
Hide resolved
|
Not sure if that is expected behavior: The mouseover text for single icons only appears, if the icon grid is expanded. |
@hansmannj this is intentional |
Test link