Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

✨ Add OSM Element Tags widget #2499

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

✨ Add OSM Element Tags widget #2499

wants to merge 3 commits into from
+248 −1

Conversation

homersimpsons
Copy link

@homersimpsons homersimpsons commented Nov 17, 2024 •
edited
Loading

Implements #2453

I based my devlopments on "OSMHistoryWidget", I may not have cleaned up everything yet.

Unresolved questions:

  • How to name this widget? "OSM Data", "OSM Element", "OSM Tags", "OSM Element Tags", other? (converted into comment) => OSM Element Tags
  • Should I "memoize" the retrieved data? => No need
  • Design improvements?
    • I tested to add borders to the table but it didn't look good.
    • I think stripped table would work (I can try to have stripped description list=
    • Maybe use a "raw" display? (just text with "tag:value\ntag2:value" (I do not think it will be easy to read)
    • How to handle long tag value? (I would say overflow ellipsis + tooltip on hover) (I did not implement this, currently ther will be a scrollbar as seen below)
    • Key on top of value? (I tested this and I do not feel like it was easier to read)

Screenshots

(With original name, not updated to "OSM Element Tags" name)

OSM Data loaded: multiple
OSM Data loaded: single
OSM Data loaded: long value
OSM Data loading
OSM Data no element
OSM Data fetch error

@homersimpsons homersimpsons force-pushed the feature/osm-data-widget branch 3 times, most recently from 35f489a to d6d1a11 Compare November 17, 2024 17:35
@homersimpsons homersimpsons marked this pull request as ready for review November 17, 2024 17:38
@homersimpsons homersimpsons force-pushed the feature/osm-data-widget branch 3 times, most recently from c048626 to e4398af Compare November 21, 2024 20:28
@homersimpsons homersimpsons force-pushed the feature/osm-data-widget branch 2 times, most recently from 4cdc531 to d4ed1cf Compare November 21, 2024 21:15
@homersimpsons
Copy link
Author

@CollinBeczak thanks for the comments. I pushed my final update, I will let you review this. Feel free to comment anything.

@homersimpsons homersimpsons force-pushed the feature/osm-data-widget branch 2 times, most recently from 0cbb694 to eace1cd Compare December 3, 2024 21:28
@homersimpsons homersimpsons changed the title ✨ Add OSM Data widget ✨ Add OSM Element Tags widget Dec 3, 2024
@homersimpsons homersimpsons force-pushed the feature/osm-data-widget branch from eace1cd to c835b26 Compare December 3, 2024 21:33
@CollinBeczak
Copy link
Collaborator

Looks good to me! The design at the moment works and looks good.

@CollinBeczak CollinBeczak self-requested a review December 9, 2024 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CollinBeczak CollinBeczak CollinBeczak approved these changes

@mvexel mvexel mvexel left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@homersimpsons @CollinBeczak @mvexel