Skip to content

add a tag diff to /(node|way|relation)/:id/history#6995

Open
stillhart wants to merge 1 commit intoopenstreetmap:masterfrom
stillhart:historydiff
Open

add a tag diff to /(node|way|relation)/:id/history#6995
stillhart wants to merge 1 commit intoopenstreetmap:masterfrom
stillhart:historydiff

Conversation

@stillhart
Copy link
Copy Markdown
Contributor

Description

I added a tag diff between change sets to highlight the changes of tags.

The diff is not expanded by default. I wanted the diff in between the change sets to avoid confusion with the given state at that time. I wanted the diff to be opt-in.
I used the default bootstrap colors which contrasts many other implementations.
I didn't want any strike through text.

Relates to #738 #1253 #4765

Screenshots

Expanded

image

Without expansion

image

How has this been tested?

  • I ran the (new) tests
    • docker compose exec web bundle exec rails test test/system/element_history_test.rb test/helpers/browse_tags_helper_test.rb
  • I tested it manually on my docker instance

What should be discussed?

  1. It won't show a diff across pagination since I didn't want to mess with record loading. This is by design at the moment. Could be added by just giving the record to the partial.
  2. Design is ok but somewhat lacking, but is using the existing bootstrap design.
  3. I tried to be screen reader friendly since colors are not obvious, but I'm not certain about that.
  4. I don't like the text Tag changes from version #1 to #2 all that much.
  5. Additions green, removals red, but modifications could also be two shades of blue. Or two other shades of green and red.
  6. Naming of the two change sets might need improvement inside the code:
    • old new
    • previous next
    • before after

None of the above seem to be deal breakers for me.

Note: I was AI assisted.

@stillhart
Copy link
Copy Markdown
Contributor Author

Just came across #6448. That one has kinda nicer styling but I don't like the "in change set view". Mine seems to be a different approach. An obvious diff that is explicitly in between the two affected records.

@tomhughes tomhughes added the ai-assisted Suggestions making extensive use of outsourced intelligence label Apr 11, 2026
@tomhughes
Copy link
Copy Markdown
Member

tomhughes commented Apr 11, 2026

As you have apparently now discovered there is already a well developed PR working on this that has had a number of rounds of review so I think it's likely that is the best place to be working on things.

@1ec5
Copy link
Copy Markdown
Collaborator

1ec5 commented Apr 12, 2026

An obvious diff that is explicitly in between the two affected records.

Maybe that could go on a separate page that’s dedicated to diffing two versions of an element, as in #6761? That could be in addition to what’s going on in #6448.

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

Labels

ai-assisted Suggestions making extensive use of outsourced intelligence

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants