Skip to content

Add previous/next entry navigation to diary pages #6901#7038

Open
Sinty026 wants to merge 4 commits into
openstreetmap:masterfrom
Sinty026:feature/diary-page-navigation
Open

Add previous/next entry navigation to diary pages #6901#7038
Sinty026 wants to merge 4 commits into
openstreetmap:masterfrom
Sinty026:feature/diary-page-navigation

Conversation

@Sinty026

@Sinty026 Sinty026 commented Apr 25, 2026

Copy link
Copy Markdown

Description

Addresses #6901 — diary pages currently have no previous/next navigation, requiring users to return to the diary overview to move between entries.

This PR adds a navigation bar above the comments section with Previous Entry and Next Entry links. The links are conditionally rendered. If there is no previous or next entry, the corresponding button is hidden.

The adjacent entries are looked up in the controller using the diary entry's ID to find the nearest entry before and after in the same user's diary.

How has this been tested?

Manually verified the nav bar renders correctly above the comments section on a diary entry page.
Three test cases were added to the diary entries controller test:

  • Middle entry shows both previous and next links
  • First entry shows only a next link
  • Last entry shows only a previous link

Screenshot

Screenshot 2026-05-16 232748

@phanq003 phanq003 force-pushed the feature/diary-page-navigation branch from d009b97 to 8bf648a Compare April 30, 2026 15:28
Sinty026 and others added 3 commits May 1, 2026 01:07
Insert a nav bar above the comments with 'Previous Entry' and 'Next Entry' they serve as UI placeholders and still need logic implemented.
Adds logic to select adjacent diary entries for a user and links
the navigation buttons accordingly.

Uses Arel predicates for previous/next queries to avoid raw SQL
and satisfy linting rules.
Add tests covering diary entry navigation behaviour,
including cases for first and last entries.

Includes minor updates to navigation markup and
missing locale keys required for tests to pass.
@phanq003 phanq003 force-pushed the feature/diary-page-navigation branch from 8bf648a to 4f763ca Compare April 30, 2026 16:01
@phanq003

Copy link
Copy Markdown

Update: The implementation now includes backend logic and tests for previous/next diary entry navigation. The PR is now fully functional and fetches adjacent entries.

@Sinty026 Sinty026 marked this pull request as ready for review May 3, 2026 10:47
@pablobm

pablobm commented May 14, 2026

Copy link
Copy Markdown
Contributor

I'll leave to maintainers to decide if this feature is desired. It makes sense to me and it's a common feature of many blogging sites, but it not strictly necessary and might be perceived as making things a bit busy.

Incidentally, for visual changes it's useful to include screenshots, so that maintainers can get a quick idea of what the change looks like before going into a proper review. Here's one:

Buttons as proposed in this PR, showing at the bottom of a diary entry, after the social links and before the comments

Visuals

The first thing that caught my attention was the style of the buttons, which is a bit different to the ones normally used for pagination. Yours have a subdued grey text and relatively heavy border, compared to the existing ones with text in standard link blue and a subdued border:

Buttons in this PR Existing pagination buttons
Buttons proposed in this PR, with a subdued grey text and relatively heavy border. Currently existing pagination buttons, with text in standard link blue and a subdued border

Looking a bit more into it, I see there's other differences, such as the use of chevrons (double angle brackets) instead of single angle brackets. I think those should be more consistent with the pagination ones.

Code organisation

In general, it's a good idea to keep controllers small. Instead of having the DB queries in the controller, I'd try to move them to DiaryEntry. These can then be easily unit-tested too.

Although that brings a little problem. Your current code has the advantage of relying of a previous filtering of invisible diary entries where appropriate. Moving the code to the model would make this a bit tricky, but I think it could be done. Instead of DiaryEntry#next, we could have DiaryEntry.first_after(entry). That should work when applied to ActiveRecord relations, such as the entries variable that you are using (entries.first_after(entry)).

@Sinty026

Copy link
Copy Markdown
Author

Thanks for the feedback! I've updated the button styling to match the existing buttons. I'll also look into moving the logic to DiaryEntry.

I do see what you mean about it feeling a bit busy. do you have any suggestions on how to make it feel less messy?

@pablobm

pablobm commented May 21, 2026

Copy link
Copy Markdown
Contributor

Not sure, but it's more a question for the maintainers. I'd want to hear their opinion.

Incidentally, I just had a look and saw a diary that could benefit from something like this: https://www.openstreetmap.org/user/pussreboots/diary In fact if instead of "Previous/Next entry" we had the title of the entry it would be great in this specific case. But don't go implement that yet. Let's wait for more feedback. For one, this example is not really representative of the typical diaries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants