[openstreetmap/openstreetmap-website] Bootstrap contextmenu (PR #6410)

[Tom Hughes]

@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";
+ at 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/3254870133 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20250922/a77f5396/attachment.htm>