[openstreetmap/openstreetmap-website] Move History and Export buttons to secondary nav (PR #5151)

[David Tsiklauri]

@nertc commented on this pull request.

Other than this one comment, I have no more questions. Functionality, routing and highlighting work as intended.

On big screens edit button seems a bit lonely because of the big gap and visual difference with other menu items. I tried adding paddings and changing colors of it, but neither of them improved visual sufficiently. It's out of scope of this PR, but, in the future, it will be great if we redesign menu to make it more aligned with the new arrangement of buttons.

Also, changing `Export` text with the export icon, may end up gaining even more space and a cleaner visual without many menu items.

Overall, this PR fixes the issue in a clean, functional way.

> @@ -412,6 +412,9 @@ $(document).ready(function () {
 
     if (OSM.router.route(this.pathname + this.search + this.hash)) {
       e.preventDefault();
+      if (this.pathname !== "/directions") {

Why do we need explicit check for the directions path? If I understand correctly, this line of code is for keeping menu opened when navigating through directions pages, but app\assets\javascripts\index\directions.js:123:5 still adds "close" class to the header when it gets routes.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5151#pullrequestreview-2302621396
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/5151/review/2302621396 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20240913/3ec2a0bb/attachment-0001.htm>