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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactoring/tobias 117 convert leaflet to js #137
Refactoring/tobias 117 convert leaflet to js #137
Changes from 16 commits
d8bf402
8a0d448
ae9162c
65fae76
a64fed5
a60da05
c29d2cf
c56ff7c
38007b1
54b2b66
4319ac8
c6f9eff
2e9980a
2265a8a
37153ec
371eeba
1a82f10
a5ad752
4af5885
75b542c
543afa2
776ae89
1eab688
8ce15e3
eda4a02
71a1e7f
e717412
63ed5ac
6138956
974f469
4443c89
b5f25c6
aa4485c
8777472
a96f51b
4a75617
2435fb9
3b597a5
08339e3
ee0035b
98d559e
06517dc
5a447c4
a3aa0ad
fac6fff
27e8287
008eecb
048d980
d162876
6705a7b
3829594
0fdfe85
6a57af1
a0aa143
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because we cannot execute Ruby code inside the JS file, right? It might be difficult to fix (and use the Ruby constant again), because afaik there is no such thing as a
.js.erb
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't really pass things from ruby over to js. We might find a workaround, where we provide this thing from the server via an ajax request (like we did with the other constants) but maybe it won't be necessary due to the new UI which uses js as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AJAX just for getting a string constant seems a bit ugly (because we need new/separate endpoints for each constant), but I'm fine with leaving this as it is until we know how the new UI and translation is implemented. We should just keep this in mind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using an Ajax call to get a string, you could also consider writing the value as a
data
attribute into an HTML element in the.erb
template, which is read by the JS ($('.map').data('location')
), i.e.<div class="map" data-location="Your location"></div>
. Then the value is bound to the element that requires it. This pattern is sometimes called 'Unobtrusive JS': https://guides.rubyonrails.org/working_with_javascript_in_rails.html#unobtrusive-javascript