pablobm left a comment (openstreetmap/openstreetmap-website#6865)
Thank you, I think this is showing promise. A few comments:
- On small screens, the sticky nav can obscure the content. This should be
responsive.
- The bottom padding of the navigation is a bit awkward. There's a bit too much
padding, then the panel content starts right after the border, with no
breathing space. I think there should be top padding on the scrollable panel,
and less bottom padding above the border. I'm open to other options.
<img width="428" height="320" alt="element-history-vertical-spacing"
src="https://github.com/user-attachments/assets/3c3e4e26-d8a5-438b-8e81-789c57816875";
/>
- What's the `z-1` class for? Perhaps it makes sense, but I don't know what it
is. I have been playing with it and can't find a difference with/without.
- The changes should also apply to the "version show" pages. Click on
"History", then click on an individual version number (eg: `Version #3`) to see
it. Template should be `app/views/old_elements/show.html.erb`.
Then there's this part of the original issue:
> The navigation bar repeats some of the information already present in the
> headings. Maybe we could consolidate this information.
But that can be done in a separate PR instead of here.
Finally, remember to present a clean commit history. From
[CONTRIBUTING.md](https://github.com/openstreetmap/openstreetmap-website/blob/master/CONTRIBUTING.md#pull-requests):
> Please ensure your commit history is clean and avoid including "fixup"
> commits. If you have added a fixup commit (for example to fix a rubocop
> warning, or because you changed your own new code) please combine the fixup
> commit into the commit that introduced the problem. git rebase -i is very
> useful for this.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/6865#issuecomment-4031642537
You are receiving this because you are subscribed to this thread.
Message ID:
<openstreetmap/openstreetmap-website/pull/6865/[email protected]>
_______________________________________________
rails-dev mailing list
[email protected]
https://lists.openstreetmap.org/listinfo/rails-dev
