@tomhughes commented on this pull request.
I think the structure of the commits here is wrong - rather than removing the
old assets and commenting out the code in the first commit and then rewriting
things and re-enabling the code in the second I would reverse things and have
the first commit rework the code and the the second one clean up the now unused
assets, style rules etc.
On a broader level I'm not sure what I think about this... Getting rid of some
non-npm vendored js is nice but I on the flip side we're moving from having
something do the work for us to seemingly doing everything from scratch
ourselves.
I think I thought bootstrap was going to be doing more for us then it actually
is - it's just providing styling and not actually any sort of popup menu
support which is more what I had assumed we were talking about.
> @@ -591,6 +591,14 @@ en:
no_such_entry:
heading: "No entry with the id: %{id}"
body: "Sorry, there is no changeset with the id %{id}. Please check your
spelling, or maybe the link you clicked is wrong."
+ context:
If you moved this to `layouts.contextmenu` instead of here then you could just
use `.directions_from` etc in the view to reference them.
> @@ -4,6 +4,9 @@
@import "rails_bootstrap_forms";
@import "colors";
+$bootstrap-icons-font-dir: "bootstrap-icons/font/fonts";
+@import "bootstrap-icons/font/bootstrap-icons";
Am I right in thinking that we've used icons from bootstrap icons before, but
by manually importing them to our assets instead of like this?
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/6410#pullrequestreview-3254870133
You are receiving this because you are subscribed to this thread.
Message ID:
<openstreetmap/openstreetmap-website/pull/6410/review/[email protected]>_______________________________________________
rails-dev mailing list
[email protected]
https://lists.openstreetmap.org/listinfo/rails-dev
