Re: [openstreetmap/openstreetmap-website] Add locale selector (PR #5201)
> Adding icon to the sprite was solely for the accordance with the current > state and practices of the project. Currently we use both `` and css sprite maps. One problem with sprite maps is that it's more difficult to control the colors of the icons. app/assets/images/sprite.svg is mainly used for buttons inside Leaflet map views, where we don't entirely control html. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5201#issuecomment-2352613716 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add the ability to redact changeset comments (Issue #5219)
- changeset comment tags: duplicate of #470 - changeset discussion comments: already implemented -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5219#issuecomment-2352641044 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add the ability to redact changeset comments (Issue #5219)
Closed #5219 as not planned. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5219#event-14271220988 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Replace creation_ip with creation_address (PR #5218)
Merged, thanks. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5218#issuecomment-2352794799 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Replace creation_ip with creation_address (PR #5218)
Merged #5218 into master. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5218#event-14272241868 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Order changeset elements for consistent pagination (PR #5209)
This was suggested in https://github.com/openstreetmap/openstreetmap-website/pull/4571#issuecomment-1994856482 but I haven't done it because we don't have appropriate indexes. This requires reading all changeset node/ways/relations and sorting them. We also have counts of changeset elements but at least counting doesn't require sorting. Also if you want the pagination here to be closer to like it is for traces, diary entries etc (https://github.com/openstreetmap/openstreetmap-website/pull/5205#issuecomment-2346879691), that's done with sorting by some id. We can't sort changeset elements just by id because ids are not unique, you can have several versions of the same element. That's why this PR sorts by (id, version) but it's not quite the same as in other places. What would have made it nearly the same is something like #4660. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5209#issuecomment-2352910968 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Remove classic pagination (PR #5205)
@AntonKhorev pushed 1 commit. 8acfc36fcb94abec9bcbce4152b830f63943a0be Remove classic pagination -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5205/files/2637e79ddbdd4f4e9e04e7b6175c3ff16f2e9ca6..8acfc36fcb94abec9bcbce4152b830f63943a0be You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Provide a truncated view of recent diaries (PR #5121)
When I thought how to do truncated diary entries I first considered doing it server-side. I already go through markdown parse trees in the richtext library to get images and descriptions for Open Graph tags. I can probably extend that to generate truncated entries. Then I thought that ultimately I want to limit entry heights. Isn't it easier to ask the browser what height each element is instead of trying to estimate it server-side? And if there's an image, it usually doesn't have its height specified in the source markdown code, therefore I can't know it server-side but client-side I can. But it's not that simple. Images can load any time, including never. I'd have to listen to load events to recalculate the heights. Now it doesn't look like a good idea. Let's suppose there's a large image and some text below. Before the image is loaded that text fits in the truncated view. Am I going to remove the text after the image loads, possibly when the user is reading it? I guess it's better to estimate heights and truncate server-side. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5121#issuecomment-2353504228 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Drop creation_ip column (PR #5212)
Merged #5212 into master. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5212#event-14290646338 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Drop creation_ip column (PR #5212)
Merged, thanks. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5212#issuecomment-2355636900 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Remove unnecessary comma from several translatable messages (PR #5220)
That comma is from ages ago https://github.com/openstreetmap/openstreetmap-website/commit/e0e849c91c66232c986a54da4a848d77f29bcb5b -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5220#issuecomment-2356278081 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add sorting and filtering functionality to user notes page (PR #5255)
@AntonKhorev commented on this pull request.
> <% else %>
+ <%= form_tag(url_for("controller" => "notes", "action" => "index"), "method"
=> :get, "data" => { "turbo" => true, "turbo-frame" => "pagination",
"turbo-action" => "advance" }) do %>
+
+
+<%= label_tag :status, t(".status") %>
+<%= select_tag :status,
+options_for_select([[t(".all"), "all"], [t(".open"),
"open"], [t(".closed"), "closed"]], params[:status]),
+:include_blank => t(".select_status"),
Why do you need `:include_blank`s? You have labels that say what these selects
are and the filter acts as if the first options are selected, but you don't
show them as selected.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255#pullrequestreview-2369155421
You are receiving this because you are subscribed to this thread.
Message ID:
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add sorting and filtering functionality to user notes page (PR #5255)
About the date pickers: I wanted to add them to our standard pagination because we already have date indexes. Then we wouldn't need them here, specifically in this notes filer. But this plan haven't worked so far because: - I wanted to first add a simpler thing to the standard pagination, namely links to first/last pages. I made several versions of this feature - #4710 #4729 #4733 #4734 - but none of them got much attention from the maintainers. I can still choose to add date pickers, but this will give me one more pull request that's going to be in conflict with the others which I'll have to maintain. - As you can see this notes list doesn't even use the standard pagination. I tried doing that too #4532. That wasn't convincing enough because it was too complicated. The complications were there because I used note ids as a cursor but I couldn't sort by them, this wouldn't get notes in the last updated order. The suggested solution by one of the maintainers is to use last comment ids instead, but it's currently being undermined by another maintainer who wants to remove opening comments. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5255#issuecomment-2413857393 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Modify the way "Friends" are added (PR #5261)
@AntonKhorev commented on this pull request. > @@ -91,7 +91,7 @@ en: support_url: Support URL allow_read_prefs: read their user preferences allow_write_prefs: modify their user preferences -allow_write_diary: create diary entries, comments and make friends +allow_write_diary: create diary entries, comments and follow users This permission is not used now but if it was, would it make sense to bundle follow/unfollow together with diary editing/commenting? Maybe just remove following from here? -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5261#pullrequestreview-2368941635 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add sorting and filtering functionality to user notes page (PR #5255)
@AntonKhorev commented on this pull request.
> <% else %>
+ <%= form_tag(url_for("controller" => "notes", "action" => "index"), "method"
=> :get, "data" => { "turbo" => true, "turbo-frame" => "pagination",
"turbo-action" => "advance" }) do %>
+
+
+<%= label_tag :status, t(".status") %>
+<%= select_tag :status,
+options_for_select([[t(".all"), "all"], [t(".open"),
"open"], [t(".closed"), "closed"]], params[:status]),
+:include_blank => t(".select_status"),
+:class => "form-select" %>
+
+
+<%= label_tag :note_type, t(".note_type") %>
I wouldn't call this a "note type". It's an interaction type between the user
and the note.
And then there's a question if this kind of filtering can be done efficiently
because here you have `INNER JOIN "note_comments"` twice.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255#pullrequestreview-2369178019
You are receiving this because you are subscribed to this thread.
Message ID:
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Remove Nominatim prefixes mention in contributors guide (PR #5260)
Merged #5260 into master. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5260#event-14655708047 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Remove Nominatim prefixes mention in contributors guide (PR #5260)
Merged, thanks. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5260#issuecomment-2413622660 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Share button should preserve the node/way/relation id when creating the URL (Issue #5252)
How do you want the html embed to behave in this case? -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5252#issuecomment-2413995449 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
[openstreetmap/openstreetmap-website] Add older/newer links in front of diary, comment, issue, block pages (PR #5262)
 You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5262 -- Commit Summary -- * Add older/newer links in front of diary, comment, issue, block pages -- File Changes -- M app/views/diary_comments/_page.html.erb (4) M app/views/diary_entries/_page.html.erb (4) M app/views/issues/_page.html.erb (5) M app/views/user_blocks/_page.html.erb (4) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5262.patch https://github.com/openstreetmap/openstreetmap-website/pull/5262.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5262 You are receiving this because you are subscribed to this thread. Message ID:___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add older/newer links in front of diary, comment, issue, block pages (PR #5262)
@AntonKhorev pushed 1 commit. 74be3f8b7c4fa192b8f08dcc6d11d88afbcf3c1a Add older/newer links in front of diary, comment, issue, block pages -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5262/files/53fb10f9cc28a7017e423baa5a09b5376ea2ed47..74be3f8b7c4fa192b8f08dcc6d11d88afbcf3c1a You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add older/newer links in front of diary, comment, issue, block pages (PR #5262)
@AntonKhorev pushed 1 commit. 0b053314fd7f427f2b5c8a0c35b583bda43ce0f3 Add older/newer links in front of diary, comment, issue, block pages -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5262/files/74be3f8b7c4fa192b8f08dcc6d11d88afbcf3c1a..0b053314fd7f427f2b5c8a0c35b583bda43ce0f3 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add locale selector (PR #5201)
Looks like the languages you get from `Language` don't quite match the ones expected by `Locale.list`. For example, we have `pt-BR` and `pt` in `Language` but `pt` and `pt-PT` in translations. As a result you can't select European Portuguese. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5201#issuecomment-2396722566 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Replace submitted note table colors with created/resolved (PR #5269)
> You say you ignore reopen events but in fact this seems to extend the concept > of submitted/created from who initially opened it to include anybody that > reopens it as well? Are you proposing to treat reopen events exactly like initial open events? That's not what we're currently doing with the blue *submitted* coloring. It only looks at the initial open event, or actually it does the equivalent thing of checking the *author* (the user who did the initial opening). -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5269#issuecomment-2422550451 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Replace submitted note table colors with created/resolved (PR #5269)
This is a test only for original open events: > comment.event == "opened" Reopen events are stored as `"reopened"`. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5269#issuecomment-2422613954 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Replace submitted note table colors with created/resolved (PR #5269)
@AntonKhorev pushed 1 commit. 7aeafbd890108734b866df1eaac8ef9b595d5cab Replace submitted note table color with created/resolved -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5269/files/4f4212abd012e1c89b5a5c20ebe5459d7dd80101..7aeafbd890108734b866df1eaac8ef9b595d5cab You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Replace submitted note table colors with created/resolved (PR #5269)
Alright, let's not think about filtering for now. Changed to look only at the last close event. I also wanted the "commented" indication not to be misleading. It includes more than commenting and this change stretches it further by including non-last close events. So I removed the claim that not colored notes were commented, although the sentence gets a bit long:  -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5269#issuecomment-2423962959 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Remove classic pagination (PR #5205)
@AntonKhorev pushed 1 commit. 45bbc3bbc6c615cb2509e08e36de018fabf987fe Remove classic pagination -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5205/files/1c414a44d49d43389b814e9ed1b3de4dab9f8135..45bbc3bbc6c615cb2509e08e36de018fabf987fe You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Remove classic pagination (PR #5205)
> These are good reasons why the paginator doesn't have to be cursor-based, but > they aren't reasons why it can't be cursor-based. So I think the > simplification of only having one type of pagination is worthwhile. Those were reasons for why this place is different, why it might be feasible not to downgrade the pagination here and why there won't necessarily be any simplification even if we downgrade it. > Since we need a well-defined order for either approach to work, I think it's > best to get the order defined and working with the cursor-based pagination. Do you want to introduce a single id or or some other field (like relation elements have) and sort by it? -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5205#issuecomment-2417177221 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Share button should preserve the node/way/relation id when creating the URL (Issue #5252)
> > How do you want the html embed to behave in this case? > > Naturally it should look just like the picture I posted. Naturally it can't look just like that because there's no header and no sidebars. The question is do you want the sidebar with the element info or do you just want it rendered with an orange line/circle? -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5252#issuecomment-2416963409 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Replace submitted note table colors with created/resolved (PR #5269)
Another option is to ignore all close events other than the last one, that's symmetrical to ignoring reopen events. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5269#issuecomment-2422639257 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Share button should preserve the node/way/relation id when creating the URL (Issue #5252)
We could have links to specific versions of elements that don't change but even the actual osm website can't show them currently. There is a pull request for nodes though #4930. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5252#issuecomment-2423835190 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Replace submitted note table colors with created/resolved (PR #5269)
There are some drawbacks in looking only at the last close event. #5255 tried to do filtering by interaction type but the db queries are ugly. To optimize them we could have a users x notes table where we store among other things interaction flags. When a user closed a note, we set a closed flag there, then we can filter notes by that flag. But if we want to pay attention only to the last close event, we'd have to clear that flag when somebody reopens the note. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5269#issuecomment-2423900130 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add social sharing functionality (PR #4985)
@AntonKhorev commented on this pull request.
> @@ -0,0 +1,27 @@
+// Opening pop-ups with share URL
+function openShareUrl(url, initialWidth = 640, initialHeight = 480) {
+ if (typeof url !== "string" || !url.startsWith("http")) {
+console.error("Invalid URL"); // Consider removing this line if console
warnings should be avoided.
+return;
+ }
+
+ const width = Math.max(100, Math.min(screen.width, initialWidth));
+ const height = Math.max(100, Math.min(screen.height, initialHeight));
+
+ const left = (screen.width / 2) - (width / 2);
+ const top = (screen.height * 0.3) - (height / 2);
+ const opts =
`width=${width},height=${height},left=${left},top=${top},menubar=no,status=no,location=no`;
+
+ window.open(url, "popup", opts);
But you're also opening a new window that's not going to contain anything and
will have to be closed manually. Or is it working differently for you?
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4985#discussion_r1809576217
You are receiving this because you are subscribed to this thread.
Message ID:
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev
[openstreetmap/openstreetmap-website] Don't show older/newer buttons if all items fit on one page (PR #5276)
You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5276 -- Commit Summary -- * Don't show older/newer buttons if all items fit on one page -- File Changes -- M app/views/shared/_pagination.html.erb (4) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5276.patch https://github.com/openstreetmap/openstreetmap-website/pull/5276.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5276 You are receiving this because you are subscribed to this thread. Message ID:___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
[openstreetmap/openstreetmap-website] Remove "User's Diary" from diary entry og:title (PR #5274)
You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5274 -- Commit Summary -- * Remove "User's Diary" from diary entry og:title -- File Changes -- M app/controllers/diary_entries_controller.rb (1) M app/helpers/open_graph_helper.rb (2) M test/controllers/diary_entries_controller_test.rb (11) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5274.patch https://github.com/openstreetmap/openstreetmap-website/pull/5274.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5274 You are receiving this because you are subscribed to this thread. Message ID:___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Always do reverse geocoding when routing (PR #5064)
Updated pull request description. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5064#issuecomment-2427309130 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
[openstreetmap/openstreetmap-website] Use system font on error pages (PR #5277)
I opened #5131 and put a number of changes to error pages to see which of them are acceptable. The discussion there however has been about making more changes. I guess that means nobody objects to the ones I made and we can have them merged. This PR changes the font of error pages to a default system ui font. Bootstrap does the same thing except also adds fallback fonts for older browsers that don't understand `font-family: system-ui`. We don't need a serif font for a text this short because we don't use serif fonts even for longer texts on the website. Addresses this from #3532: > It also looks incredibly ugly, with loads of whitespace and the default browser font.  You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5277 -- Commit Summary -- * Use system font on error pages -- File Changes -- M app/assets/stylesheets/errors.scss (4) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5277.patch https://github.com/openstreetmap/openstreetmap-website/pull/5277.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5277 You are receiving this because you are subscribed to this thread. Message ID:___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
[openstreetmap/openstreetmap-website] Display old node versions on map view (PR #5278)
Shows node versions like in #4930 but without complications https://github.com/openstreetmap/openstreetmap-website/pull/4930#issuecomment-2198493197 and without saving api calls.  You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5278 -- Commit Summary -- * Display old node versions on map view -- File Changes -- M app/assets/javascripts/index.js (14) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5278.patch https://github.com/openstreetmap/openstreetmap-website/pull/5278.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5278 You are receiving this because you are subscribed to this thread. Message ID:___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Use system font on error pages (PR #5277)
> My PR hasn't been merged in two months so it must be good is an interesting > take. How else can I get changes from that PR to be discussed instead of being asked to make more changes like: > Is there a reason not to use bootstrap here? Yes https://github.com/openstreetmap/openstreetmap-website/pull/5130#discussion_r1738684207 -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5277#issuecomment-2427830659 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add sorting and filtering functionality to user notes page (PR #5255)
> As for the next steps, I’m not sure whether it’s better to get this PR merged > as is and refactor later when the notes move to standard pagination, or if we > should address the pagination issue on user note pages first and then > refactor this PR accordingly. You can make a pull request that just adds the status filter. That doesn't depend on pagination and shouldn't tank the performance because: - status is checked anyway to see if the note is not hidden - we're doing a similar thing in the api -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5255#issuecomment-2419119591 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add sorting and filtering functionality to user notes page (PR #5255)
> On a side note, I’ve managed to implement sortable columns on a table UI end > for created_at and updated_at locally I looked again at why similar things were rejected in the past like #1656 and there doesn't seem to be any clear reason. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5255#issuecomment-2419161015 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
[openstreetmap/openstreetmap-website] Replace submitted note table colors with created/resolved (PR #5269)
Instead of *submitted* and *commented* note colors switch to *created*, *resolved* and *commented*. Reasons: - Close actions were colored as *commented* even if notes were closed without any comments. Now they are going to be colored as *resolved*. - *Submitted* is not a term we use for notes, we have "add a note" and notes "created by". - There was a [complaint](https://github.com/openstreetmap/openstreetmap-website/pull/4700#issuecomment-2221682594) about the primary color used for submitted notes that is blue making links harder to see. What other color can we use? For example, the *success* color, like we do for unread messages. But that's the color of the resolved note marker, and then we might as well add the unresolved color. Caveats: - I keep "submitted" in locale keys to avoid errors while `subheading_html` is not updated. - I ignore *reopen* events because if I don't, I'd have to treat them as *open* events but then there wouldn't be an equivalent to the previous *submitted* state. And the equivalent is green *created* + yellow *created and resolved*. - I had to add this *created and resolved* state because what if the user did both actions? I can't pick one more important color. If I look at note status changes, the latest action is the most important one and I'd have to color notes as *resolved*. But then again I'd lose the equivalent to the previous *submitted* state.  Dark mode:  Legend while locale strings aren't updated:  You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5269 -- Commit Summary -- * Replace submitted note table color with created/resolved -- File Changes -- M app/assets/stylesheets/common.scss (8) M app/views/notes/index.html.erb (22) M config/locales/en.yml (6) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5269.patch https://github.com/openstreetmap/openstreetmap-website/pull/5269.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5269 You are receiving this because you are subscribed to this thread. Message ID:___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add sorting and filtering functionality to user notes page (PR #5255)
@AntonKhorev commented on this pull request.
> @@ -6,41 +7,69 @@
:commented => tag.span(t(".subheading_commented"), :class => "px-2
py-1 bg-body") %>
<% end %>
-<% if @notes.empty? %>
- <%= t ".no_notes" %>
+<%= form_tag(url_for("controller" => "notes", "action" => "index"), "method"
=> :get, "data" => { "turbo" => true, "turbo-frame" => "pagination",
"turbo-action" => "advance" }) do %>
+
+
+ <%= label_tag :status, t(".status") %>
+ <%= select_tag :status,
+ options_for_select([[t(".all"), "all"], [t(".open"),
"open"], [t(".closed"), "closed"]], params[:status] || "all"),
+ :class => "form-select",
+ :onchange => "this.form.requestSubmit()" %>
`onchange` is not going to work with the `csp_enforce: true` setting.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5255#pullrequestreview-2374797957
You are receiving this because you are subscribed to this thread.
Message ID:
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add sorting and filtering functionality to user notes page (PR #5255)
Besides the [double-join db query](https://github.com/openstreetmap/openstreetmap-website/pull/5255#discussion_r1801064970), there's couple more issues with the interaction type: ["blue on blue" colors](https://github.com/openstreetmap/openstreetmap-website/pull/4700#issuecomment-2221682594) and whether the submitted/commented classification makes sense at all (what if I closed a note without a comment?). I wanted to try opened/closed/commented instead. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5255#issuecomment-2419194257 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add locale selector (PR #5201)
@AntonKhorev commented on this pull request.
> @@ -0,0 +1,16 @@
+
+
+
+
+ <% unless disabled %>
+
+ <% Locale.available
+ .map { |locale| Language.find_by(:code => locale.to_s) }
You'll get a ton of db queries:
```
Language Load (0.2ms) SELECT "languages".* FROM "languages" WHERE
"languages"."code" = $1 LIMIT $2 [["code", "af"], ["LIMIT", 1]]
↳ app/views/shared/_language_selector.html.erb:8
Language Load (0.3ms) SELECT "languages".* FROM "languages" WHERE
"languages"."code" = $1 LIMIT $2 [["code", "ar"], ["LIMIT", 1]]
↳ app/views/shared/_language_selector.html.erb:8
Language Load (0.3ms) SELECT "languages".* FROM "languages" WHERE
"languages"."code" = $1 LIMIT $2 [["code", "az"], ["LIMIT", 1]]
↳ app/views/shared/_language_selector.html.erb:8
Language Load (0.3ms) SELECT "languages".* FROM "languages" WHERE
"languages"."code" = $1 LIMIT $2 [["code", "be"], ["LIMIT", 1]]
↳ app/views/shared/_language_selector.html.erb:8
Language Load (0.3ms) SELECT "languages".* FROM "languages" WHERE
"languages"."code" = $1 LIMIT $2 [["code", "bg"], ["LIMIT", 1]]
↳ app/views/shared/_language_selector.html.erb:8
...
```
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5201#pullrequestreview-2375219073
You are receiving this because you are subscribed to this thread.
Message ID:
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Display old node versions on map view (PR #5278)
> I believe similar changes to remaining OSM.OldBrowse() occurrences will > enable displaying old ways / relations versions? :-) As soon as https://github.com/openstreetmap/openstreetmap-website/pull/4930#issuecomment-2198793487 is done. > Q: I found one strange behavior with nodes / ways / relations pagination > controls. They initially display "Version 1" and "last version", although > they should perhaps display only "previous version" link initially. But before that thing is done, *current version without a number* and *version with the last number* are really different pages, although the UI is probably not the best here. See also #4946. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5278#issuecomment-2430502067 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add social sharing functionality (PR #4985)
@AntonKhorev commented on this pull request.
> @@ -0,0 +1,27 @@
+// Opening pop-ups with share URL
+function openShareUrl(url, initialWidth = 640, initialHeight = 480) {
+ if (typeof url !== "string" || !url.startsWith("http")) {
+console.error("Invalid URL"); // Consider removing this line if console
warnings should be avoided.
+return;
+ }
+
+ const width = Math.max(100, Math.min(screen.width, initialWidth));
+ const height = Math.max(100, Math.min(screen.height, initialHeight));
+
+ const left = (screen.width / 2) - (width / 2);
+ const top = (screen.height * 0.3) - (height / 2);
+ const opts =
`width=${width},height=${height},left=${left},top=${top},menubar=no,status=no,location=no`;
+
+ window.open(url, "popup", opts);
Here's how it looks for me:
I get a blank window in background and a new tab.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4985#discussion_r1810547849
You are receiving this because you are subscribed to this thread.
Message ID:
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add social sharing functionality (PR #4985)
@AntonKhorev commented on this pull request.
> @@ -0,0 +1,27 @@
+// Opening pop-ups with share URL
+function openShareUrl(url, initialWidth = 640, initialHeight = 480) {
+ if (typeof url !== "string" || !url.startsWith("http")) {
+console.error("Invalid URL"); // Consider removing this line if console
warnings should be avoided.
+return;
+ }
+
+ const width = Math.max(100, Math.min(screen.width, initialWidth));
+ const height = Math.max(100, Math.min(screen.height, initialHeight));
+
+ const left = (screen.width / 2) - (width / 2);
+ const top = (screen.height * 0.3) - (height / 2);
+ const opts =
`width=${width},height=${height},left=${left},top=${top},menubar=no,status=no,location=no`;
+
+ window.open(url, "popup", opts);
What happens if you replace your document ready listener with this?
```js
$(document).ready(function () {
$(".ssb-icon").on("click", function (e) {
const shareUrl = $(this).attr("href");
if (!shareUrl.startsWith("mailto:";)) {
e.preventDefault();
openShareUrl(shareUrl);
}
});
});
```
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4985#discussion_r1810571206
You are receiving this because you are subscribed to this thread.
Message ID:
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Modify the way "Friends" are added (PR #5261)
@AntonKhorev commented on this pull request. > @@ -280,8 +280,8 @@ resource :profile, :only => [:edit, :update] # friendships - match "/user/:display_name/make_friend" => "friendships#make_friend", :via => [:get, :post], :as => "make_friend" - match "/user/:display_name/remove_friend" => "friendships#remove_friend", :via => [:get, :post], :as => "remove_friend" + match "/user/:display_name/follow_user" => "friendships#follow_user", :via => [:get, :post], :as => "follow_user" + match "/user/:display_name/unfollow_user" => "friendships#unfollow_user", :via => [:get, :post], :as => "unfollow_user" > `app/views/user_mailer/friendship_notification.html.erb` and > `app/views/user_mailer/friendship_notification.text.erb` changes are about > redirections from mail notifications. I don't see any redirects from old GET paths to new GET paths (but I see changes in tests that checked the paths). > For the consistent visual output this PR is focused on changing every > occurrence of "friend" on user side and changing them to "following". Main > focus is what will users see (including both texts and URLs). Maybe it shouldn't focus on *both*? -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5261#discussion_r1814908014 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add filtering functionality to user notes page (PR #5255)
> Adding filters one by one would significantly increase the time required Time for what? For PR review? > and might leave the feature feeling incomplete. Are you sure? Is it complete with exactly the filters you added and not some other filters? -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5255#issuecomment-2435272945 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Note subscriptions (PR #5283)
@AntonKhorev pushed 6 commits. 51187ec2e0bdbe0ae728f4eb37250c4394e376f4 Subscribe users when they interact with notes d67bca8a39a12876c2ab265b0dfdcbf61394c652 Backfill note subscriptions f91f74db2e7fc206ff8a7f349e33f34a1f0871a4 Send notifications to note subscribers instead of commenters fc9ec45bbf10b88c3acab0f9529b3c41f131b45e Add create note subscription api endpoint 0837c289274c8bd7f5434e29fdaf1f7c97a7a552 Add create note subscription api endpoint 1de0127518d9afcc7d02919f9548eefef8f6729e Add subscribe/unsubscribe buttons to note pages -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5283/files/3b1657c342809c4395ac5c92699962fd8a5ba715..1de0127518d9afcc7d02919f9548eefef8f6729e You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
[openstreetmap/openstreetmap-website] Note subscriptions db table (PR #5284)
Part one of #5283. Creates the note subscription table and starts adding subscriptions for anyone who comments a note. These subscriptions are not yet used for anything, email notifications still go to all note commenters. You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5284 -- Commit Summary -- * Create note subscription table and model * Subscribe users when they interact with notes -- File Changes -- M app/controllers/api/notes_controller.rb (2) M app/models/note.rb (2) A app/models/note_subscription.rb (20) M app/models/user.rb (2) A db/migrate/20241022141247_create_note_subscriptions.rb (8) M db/structure.sql (42) M test/controllers/api/notes_controller_test.rb (115) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5284.patch https://github.com/openstreetmap/openstreetmap-website/pull/5284.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5284 You are receiving this because you are subscribed to this thread. Message ID:___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add filtering functionality to user notes page (PR #5255)
> 3. When you mentioned "blue on blue" colors I'm not sure if this is something > I've introduced in this PR Obviously you didn't introduce it. That comment was made in July. > since when I toggle dark mode on my end everything is looking fine. Check it when the note is colored as "submitted". -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5255#issuecomment-2435264671 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add filtering functionality to user notes page (PR #5255)
> Yes, the goal was to implement the filters that were most commonly requested > and improve the feature based on that feedback. Where are these requests? For example #832 wants status filter but doesn't say anything about date ranges. Why would a status filter be incomplete without a date range filter? -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5255#issuecomment-2435347487 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Note subscriptions (PR #5283)
@AntonKhorev pushed 7 commits. e60511f711f36dd87cc8d49cedaec97241db6dbe Create note subscription table and model 9fb8291c9cca0aa97af992c20bd9a908f1a31b3d Subscribe users when they interact with notes 26c426b968f2e80f9845300458d5c15b85a158d6 Backfill note subscriptions cba78ed1105c056a38a1232af3defab69c6a37b2 Send notifications to note subscribers instead of commenters eae7129487c0cc5afad621dca79260c5bfdd91a4 Add create note subscription api endpoint 5dea7caabb6b2049454cf3756b594108b5d1100a Add create note subscription api endpoint 9bd3f067f2ae634fd8cfc29bd1136789ef091ae2 Add subscribe/unsubscribe buttons to note pages -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5283/files/f79120ac3feefa0546b9c2f2b8cded41ddd13e55..9bd3f067f2ae634fd8cfc29bd1136789ef091ae2 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Note subscriptions db table (PR #5284)
@AntonKhorev pushed 2 commits. 001fed4fd7d321a079568f5de9479adfbff21778 Create note subscription table and model 2d7e0a397a1b2dbed74f40439c1305b1b48a063e Subscribe users when they interact with notes -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5284/files/954e7b31e1ec0f47ad7000bf2a9a32af9206abd1..2d7e0a397a1b2dbed74f40439c1305b1b48a063e You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Display old node versions on map view (PR #5278)
@AntonKhorev pushed 1 commit. 0c237c11d2c4b19d1a14324adbb7ca275c554816 Display old node versions on map view -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5278/files/55794cf4cbe16d0b359da647e1a53bdd390a53dd..0c237c11d2c4b19d1a14324adbb7ca275c554816 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Use system font on error pages (PR #5277)
@AntonKhorev pushed 1 commit. b3f930095408c7678cbd000f6f125463ed462f02 Use system font on error pages -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5277/files/733e160872dfe92230645eb6e7e8f1cf9d221597..b3f930095408c7678cbd000f6f125463ed462f02 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Remove classic pagination (PR #5205)
@AntonKhorev pushed 1 commit. cc319be3414e2d2b94cb10c871cc66b541d87a08 Remove classic pagination -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5205/files/45bbc3bbc6c615cb2509e08e36de018fabf987fe..cc319be3414e2d2b94cb10c871cc66b541d87a08 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Replace submitted note table colors with created/resolved (PR #5269)
@AntonKhorev pushed 1 commit. 837aeea405129719503141350d8b669c03cbd4a3 Replace submitted note table color with created/resolved -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5269/files/7aeafbd890108734b866df1eaac8ef9b595d5cab..837aeea405129719503141350d8b669c03cbd4a3 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Note subscriptions db table (PR #5284)
@AntonKhorev commented on this pull request. > @@ -401,6 +401,8 @@ def add_comment(note, text, event, notify: true) note.comments.map(&:author).uniq.each do |user| UserMailer.note_comment_notification(comment, user).deliver_later if notify && user && user != current_user && user.visible? end + + NoteSubscription.find_or_create_by(:note => note, :user => current_user) if current_user Try running Rubocop. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5284#discussion_r1818223016 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
[openstreetmap/openstreetmap-website] Use resourceful routes for granting/revoking user roles (PR #5293)
Replaces: - `post grant_role_path` with `put user_role_path` - `post revoke_role_path` with `delete user_role_path` You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5293 -- Commit Summary -- * Use resourceful routes for granting/revoking user roles -- File Changes -- M app/abilities/ability.rb (2) M app/controllers/concerns/user_methods.rb (5) M app/controllers/user_roles_controller.rb (8) M app/helpers/user_roles_helper.rb (8) M config/routes.rb (8) M test/controllers/user_roles_controller_test.rb (46) M test/helpers/user_roles_helper_test.rb (36) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5293.patch https://github.com/openstreetmap/openstreetmap-website/pull/5293.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5293 You are receiving this because you are subscribed to this thread. Message ID:___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Use resourceful routes for granting/revoking user roles (PR #5293)
@AntonKhorev pushed 1 commit. b24d52d5ca30e081ec799880bee6bc32616ad8fa Use resourceful routes for granting/revoking user roles -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5293/files/19da04fa8eb0ce3d665123048f6cb1f62058c369..b24d52d5ca30e081ec799880bee6bc32616ad8fa You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Note subscriptions (PR #5283)
@AntonKhorev pushed 1 commit. 6fbca51ccdc1df0ecf32ca0892d2f1762d7811e7 Add subscribe/unsubscribe buttons to note pages -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5283/files/fe7031a6448c291dde482f4b5bc98a855ab2449a..6fbca51ccdc1df0ecf32ca0892d2f1762d7811e7 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
[openstreetmap/openstreetmap-website] Decrease padding between close button and inputs in directions form (PR #5288)
That space on the top of direction forms is too wasteful. Before/after:   You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5288 -- Commit Summary -- * Decrease padding between close button and inputs in directions form -- File Changes -- M app/views/layouts/_search.html.erb (2) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5288.patch https://github.com/openstreetmap/openstreetmap-website/pull/5288.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5288 You are receiving this because you are subscribed to this thread. Message ID:___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Note subscriptions (PR #5283)
@AntonKhorev pushed 1 commit. f79120ac3feefa0546b9c2f2b8cded41ddd13e55 Add subscribe/unsubscribe buttons to note pages -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5283/files/df53ca081757f8a2f6e3945df43d6087941848eb..f79120ac3feefa0546b9c2f2b8cded41ddd13e55 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
[openstreetmap/openstreetmap-website] Remove callback from initialize() in note js controller (PR #5289)
https://github.com/openstreetmap/openstreetmap-website/commit/a796c41881c26e6da8828a71dbfb533613e5a83f added a callback to `function initialize()` in `note.js`. It's called at the very end of the function. Not scheduled in some event, not skipped by early termination, nothing runs after it. Passing a callback to `initialize()` is the same as running `initialize(); callback();` You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5289 -- Commit Summary -- * Remove callback from initialize() in note js controller -- File Changes -- M app/assets/javascripts/index/note.js (22) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5289.patch https://github.com/openstreetmap/openstreetmap-website/pull/5289.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5289 You are receiving this because you are subscribed to this thread. Message ID:___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Directions need to say "go straight onto road B" (Issue #5280)
This is the Graphhopper instructions for the query:  None of the engines mentions "19" in their instructions. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5280#issuecomment-2432066814 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Modify the way "Friends" are added (PR #5261)
@AntonKhorev commented on this pull request. > @@ -280,8 +280,8 @@ resource :profile, :only => [:edit, :update] # friendships - match "/user/:display_name/make_friend" => "friendships#make_friend", :via => [:get, :post], :as => "make_friend" - match "/user/:display_name/remove_friend" => "friendships#remove_friend", :via => [:get, :post], :as => "remove_friend" + match "/user/:display_name/follow_user" => "friendships#follow_user", :via => [:get, :post], :as => "follow_user" + match "/user/:display_name/unfollow_user" => "friendships#unfollow_user", :via => [:get, :post], :as => "unfollow_user" > This PR is already +104 -104 line change. You could have a smaller PR if you only changed the texts but not the routes. Don't we have the existing urls published in notification emails (and maybe in other places)? I don't see redirects from old to new here. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5261#discussion_r1812771031 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add locale selector (PR #5201)
@AntonKhorev commented on this pull request.
> + <% Locale.available
+ .map { |locale| Language.find_by(:code => locale.to_s) }
+ .select { |locale| locale }
+ .sort_by { |locale| locale[:english_name] }
+ .each do |language| %>
+><%= language.name %>
> Translating language names using i18n and not having one source for them
i18n is going to be their *one source*
> may cause duplicate translations or inconsistency (because of multiple
> functionalities using language names).
We already have two different functionalities using two different sets of
languages: i18n and languages of diary entries.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5201#discussion_r1812724030
You are receiving this because you are subscribed to this thread.
Message ID:
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add user block api call (PR #4301)
@AntonKhorev commented on this pull request. > @@ -12,5 +15,33 @@ def show rescue ActiveRecord::RecordNotFound raise OSM::APINotFoundError end + +def create + raise OSM::APIBadUserInput, "No user was given" unless params[:user] + + user = User.visible.find_by(:id => params[:user]) + raise OSM::APINotFoundError unless user + raise OSM::APIBadUserInput, "No reason was given" unless params[:reason] + raise OSM::APIBadUserInput, "No period was given" unless params[:period] + + period = Integer(params[:period], :exception => false) + raise OSM::APIBadUserInput, "Period should be a number of hours" unless period Block durations are not stored directly. Dropdowns show the closes value to the remaining time: https://github.com/openstreetmap/openstreetmap-website/pull/5011 -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/4301#discussion_r1818199296 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Note subscriptions db table (PR #5284)
@AntonKhorev commented on this pull request. > +end + end + assert_response :success + js = ActiveSupport::JSON.decode(@response.body) + assert_not_nil js + assert_equal "Feature", js["type"] + assert_equal "Point", js["geometry"]["type"] + assert_equal [-1.0, -1.0], js["geometry"]["coordinates"] + assert_equal "open", js["properties"]["status"] + assert_equal 1, js["properties"]["comments"].count + assert_equal "opened", js["properties"]["comments"].last["action"] + assert_equal "This is a comment", js["properties"]["comments"].last["text"] + assert_equal user.display_name, js["properties"]["comments"].last["user"] + + note = Note.last + subscription = NoteSubscription.last > This ... is relying on nothing else having created notes There is a check `assert_difference "Note.count", 1` > (and all the other similar versions in other tests) Other tests in this file or other tests elsewhere? You can find similar uses of `.last` from years ago. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5284#discussion_r1818205543 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Note subscriptions db table (PR #5284)
@AntonKhorev commented on this pull request. > @@ -401,6 +401,8 @@ def add_comment(note, text, event, notify: true) note.comments.map(&:author).uniq.each do |user| UserMailer.note_comment_notification(comment, user).deliver_later if notify && user && user != current_user && user.visible? end + + NoteSubscription.find_or_create_by(:note => note, :user => current_user) if current_user That would require one more `&.` after `note__subscriptions`, so it depends on whether you think that chained `&.`s are simplifications. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5284#discussion_r1818207830 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
[openstreetmap/openstreetmap-website] Don't update deactivates_at if block was already viewed (PR #5312)
When a blocked user visits their block page, its deactivation time is recorded. Problem is this only needs to happen if there's no deactivation time set. If it's already set, it should be kept at what it is. The possible bug is that if the user visits their block page after the block is lifted, that visit time gets into `deactivates_at` and is reported as the block end time despite the block ending earlier. You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5312 -- Commit Summary -- * Don't update deactivates_at if block was already viewed -- File Changes -- M app/controllers/user_blocks_controller.rb (2) M test/controllers/user_blocks_controller_test.rb (70) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5312.patch https://github.com/openstreetmap/openstreetmap-website/pull/5312.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5312 You are receiving this because you are subscribed to this thread. Message ID:___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add profile location (PR #5302)
@AntonKhorev commented on this pull request.
> @@ -178,6 +220,42 @@ $(document).ready(function () {
}
}
+ function updateHomeLocation() {
+const lat = $("#home_lat").val().trim();
+const lon = $("#home_lon").val().trim();
+if (!lat || !lon) {
+ return;
+}
+
+const geocodeUrl =
`${OSM.NOMINATIM_URL}reverse?format=json&lat=${lat}&lon=${lon}`;
+
+if (locationInput.request) locationInput.request.abort();
+locationInput.request = $.getJSON(geocodeUrl, function (data) {
blocked by content security policy
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5302#pullrequestreview-2427085813
You are receiving this because you are subscribed to this thread.
Message ID:
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add profile location (PR #5302)
@AntonKhorev commented on this pull request. > + +<%= t ".location_name_warning" %> + + This looks like a sentence and it's not obvious that it ends with a button I'm supposed to click to update the value:  -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5302#pullrequestreview-2427111552 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Provide a truncated view of recent diaries (PR #5121)
@AntonKhorev commented on this pull request. > @@ -2,7 +2,7 @@ <%= render :partial => "diary_entry_heading", :object => diary_entry, :as => "diary_entry" %> -<%= diary_entry.body.to_html %> +<%= truncated ? diary_entry.truncated_body(1000).to_html : diary_entry.body.to_html %> First markdown from the RichText object is converted to html. Then html is processed by `truncated_body` and a new RichText object is constructed. Then html is again extracted from that object. And if the html is nontrivial it may get mangled in the process, turning, for example, this ``` There's a table below: | thing | 42 | an important thing | | thing | 43 | an important thing | ```  into this  -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5121#pullrequestreview-2427014169 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Provide a truncated view of recent diaries (PR #5121)
@AntonKhorev commented on this pull request. > + def truncate_html(html, max_length, empty_tag_length = 500) +doc = Nokogiri::HTML::DocumentFragment.parse(html) +accumulated_length = 0 +truncated_node = nil + +doc.traverse do |node| + if accumulated_length >= max_length +node.remove unless truncated_node.ancestors.include?(node) +next + end + + next unless node.children.empty? + + content_length = node.text? ? node.text.length : empty_tag_length + if accumulated_length + content_length >= max_length +node.content = node.text.truncate(max_length - accumulated_length) if node.text? Do we really need to truncate node contents rather than keeping/discarding it as a whole? -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5121#pullrequestreview-2427017460 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add profile location (PR #5302)
@AntonKhorev commented on this pull request. > @@ -0,0 +1,22 @@ +require "application_system_test_case" + +class UserLocationChangeTest < ApplicationSystemTestCase + def setup +stub_request(:get, /.*gravatar.com.*d=404/).to_return(:status => 404) + end + + test "User can change their location" do +user = create(:user) +sign_in_as(user) + +visit user_path(user.display_name) You write it differently below ```suggestion visit user_path(user) ``` -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5302#pullrequestreview-2427073474 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add profile location (PR #5302)
@AntonKhorev commented on this pull request.
> @@ -178,6 +220,42 @@ $(document).ready(function () {
}
}
+ function updateHomeLocation() {
+const lat = $("#home_lat").val().trim();
+const lon = $("#home_lon").val().trim();
+if (!lat || !lon) {
+ return;
+}
+
+const geocodeUrl =
`${OSM.NOMINATIM_URL}reverse?format=json&lat=${lat}&lon=${lon}`;
If you only want a country why don't you ask for a country
```suggestion
const geocodeUrl =
`${OSM.NOMINATIM_URL}reverse?format=json&lat=${lat}&lon=${lon}&zoom=3`;
```
instead of getting a full address of things you don't care about and then doing
`.split(",")`?
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5302#pullrequestreview-2427159022
You are receiving this because you are subscribed to this thread.
Message ID:
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Show "reporting user" on "issues" screen (PR #4990)
I don't think tooltips were a good idea. You can't interact with the usernames they show. And truncation works fine for individual names, but then you'd have to do it inside the tooltip too. Do you want to make html tooltips? And then there's Turbo cleanup.  -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/4990#issuecomment-2468193271 You are receiving this because you are subscribed to this thread. Message ID:___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Show "reporting user" on "issues" screen (PR #4990)
I wouldn't try to do a comma-separated list > UserA, UserBB, UserC I'd write them each on a new line and add truncation > UserA > UserB... > UserC And if there's more than say 3, write something like > UserA > UserB... > UserC > and 2 more users -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/4990#issuecomment-2468202474 You are receiving this because you are subscribed to this thread. Message ID:___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add the ability to redact changeset comments (Issue #5219)
Actually this issue isn't exactly a duplicate of #470. #470 asks for the ability of users to edit tags of their own changesets. And this issue here asks for the ability of moderators to delete tags of any changesets. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5219#issuecomment-2466159853 You are receiving this because you are subscribed to this thread. Message ID:___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add the ability to redact changeset comments (Issue #5219)
Reopened #5219. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5219#event-15237817121 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Note subscriptions (PR #5283)
@AntonKhorev pushed 7 commits. 0b9c7c13a1dad7b1528438150f0a94720b2f4323 Create note subscription table and model 954e7b31e1ec0f47ad7000bf2a9a32af9206abd1 Subscribe users when they interact with notes 9bbab0028a9cb40c373af3f97c68ef2f772258ed Backfill note subscriptions 1fba9ddd0dd1f709762c67a3f042e2dacec246dc Send notifications to note subscribers instead of commenters 4c83e68c4f123942da37b7a711b53f118bc66631 Add create note subscription api endpoint a3eabffb18cf46a8ef14bdf04f93deaffbcf26ce Add create note subscription api endpoint fe7031a6448c291dde482f4b5bc98a855ab2449a Add subscribe/unsubscribe buttons to note pages -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5283/files/1de0127518d9afcc7d02919f9548eefef8f6729e..fe7031a6448c291dde482f4b5bc98a855ab2449a You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Note subscriptions (PR #5283)
@AntonKhorev pushed 1 commit. df53ca081757f8a2f6e3945df43d6087941848eb Add subscribe/unsubscribe buttons to note pages -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5283/files/6fbca51ccdc1df0ecf32ca0892d2f1762d7811e7..df53ca081757f8a2f6e3945df43d6087941848eb You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
[openstreetmap/openstreetmap-website] Note subscriptions API (PR #5314)
Part 4 of #5283 Adds API endpoints to subscribe/unsubscribe from a note. This is like the changeset subscribe/unsubscribe https://wiki.openstreetmap.org/wiki/API_v0.6#Subscribe:_POST_/api/0.6/changeset/#id/subscribe but since we like resourceful routes, instead of `POST .../subscribe` and `POST .../unsubscribe`, this PR adds: - `POST /api/0.6/notes/:id/subscription` to add a subscription - `DELETE /api/0.6/notes/:id/subscription` to remove a subscription You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5314 -- Commit Summary -- * Add create note subscription api endpoint * Add destroy note subscription api endpoint -- File Changes -- M app/abilities/api_capability.rb (1) A app/controllers/api/note_subscriptions_controller.rb (22) M config/routes.rb (2) A test/controllers/api/note_subscriptions_controller_test.rb (132) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5314.patch https://github.com/openstreetmap/openstreetmap-website/pull/5314.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5314 You are receiving this because you are subscribed to this thread. Message ID:___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Don't update deactivates_at if block was already viewed (PR #5312)
@AntonKhorev pushed 1 commit. ff0569829f81f868e054e6430eaf11bf779d1213 Don't update deactivates_at if block was already viewed -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5312/files/f4208862bc1841dc18aa55ce85acad349b6f2b43..ff0569829f81f868e054e6430eaf11bf779d1213 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Don't update deactivates_at if block was already viewed (PR #5312)
@AntonKhorev commented on this pull request. > + get user_block_path(block) + assert_response :success + block.reload + assert_not block.needs_view + assert_equal Time.now.utc - 1.hour, block.deactivates_at +end + end + + ## + # test clearing needs_view by showing a timed block to the blocked user + def test_show_sets_deactivates_at_for_timed_block +user = create(:user) +session_for(user) + +freeze_time do + block = create(:user_block, :needs_view, :created_at => Time.now.utc, :ends_at => Time.now.utc + 1.day, :user => user) updated, but if it's actually an issue, `test/factories/user_blocks.rb` still uses days -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5312#discussion_r1838884712 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Note subscriptions (PR #5283)
@AntonKhorev pushed 3 commits. 3ecf7943b36836b9f0337aa9d7c8f48992293ae8 Add create note subscription api endpoint bc8c005d6186c56054fc68d8281e67ba8efb76cc Add destroy note subscription api endpoint 98a287457e6742a9e625f66b7acbdb8898304737 Add subscribe/unsubscribe buttons to note pages -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5283/files/ffdeb4b12644e3d5f8b1adc916e20418981a74c6..98a287457e6742a9e625f66b7acbdb8898304737 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
[openstreetmap/openstreetmap-website] Remove unused timer from new note js controller (PR #5316)
https://github.com/openstreetmap/openstreetmap-website/commit/51dcf86f40200bf4728097868f45aa2fe12968f3 added the "removenote" timer. Its use was removed in https://github.com/openstreetmap/openstreetmap-website/commit/d380ce79aabcd98ade3eabe8f982e516311f6aee but the `stopTime` cleanup stuck. You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5316 -- Commit Summary -- * Remove unused timer from new note js controller -- File Changes -- M app/assets/javascripts/index/new_note.js (2) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5316.patch https://github.com/openstreetmap/openstreetmap-website/pull/5316.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5316 You are receiving this because you are subscribed to this thread. Message ID:___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
[openstreetmap/openstreetmap-website] Css spinner delay (PR #5315)
Similarly to #5307, we can avoid scheduling/unscheduling js events to show spinners in the richtext preview #5311 and the sidebar loader:  There's a delay to avoid blinking on fast loads, that delay can be implemented using a css animation. And we also can stop using `vendor/assets/jquery/jquery.timers.js` which does some ill-advised things like [listening to `unload`](https://developer.mozilla.org/en-US/docs/Web/API/Window/unload_event). You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5315 -- Commit Summary -- * Show spinner using delayed css animation in richtext preview * Show spinner using delayed css animation in sidebar loader -- File Changes -- M app/assets/javascripts/index.js (11) M app/assets/javascripts/richtext.js (9) M app/assets/stylesheets/common.scss (12) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5315.patch https://github.com/openstreetmap/openstreetmap-website/pull/5315.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5315 You are receiving this because you are subscribed to this thread. Message ID:___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
[openstreetmap/openstreetmap-website] Show "node also part of ways" as nested lists on way pages (PR #5317)
Before:  After:  You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5317 -- Commit Summary -- * Show "node also part of ways" as nested lists on way pages * Put nested "also..." lists into collapsible details -- File Changes -- M app/views/browse/_way.html.erb (25) M config/locales/en.yml (6) M test/controllers/ways_controller_test.rb (124) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5317.patch https://github.com/openstreetmap/openstreetmap-website/pull/5317.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5317 You are receiving this because you are subscribed to this thread. Message ID:___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add locale selector (PR #5201)
> The only downside of the current solution is that there will be only those languages shown, which have shared.language_selector.language key. Those that have translations that are maintained, so not much of a downside. I also had a more defensive version of that key with `language: THIS LANGUAGE NAME` but maybe it's not required. It's possible to document strings on translatewiki like this https://translatewiki.net/wiki/Osm:Site.copyright.native.native_link/qqq -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5201#issuecomment-2465065075 You are receiving this because you are subscribed to this thread. Message ID:___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add locale selector (PR #5201)
@AntonKhorev commented on this pull request.
> @@ -76,6 +85,15 @@
<%= link_to t("layouts.copyright"), copyright_path, :class =>
"dropdown-item" %>
<%= link_to t("layouts.help"), help_path, :class =>
"dropdown-item" %>
<%= link_to t("layouts.about"), about_path, :class =>
"dropdown-item" %>
+
+<% if current_user && current_user.id %>
+ <%= link_to(preferences_path) do %>
+<%= render "shared/language_selector", :hoverable => true,
:black => true, :classes => "dropdown-item", :disabled => true %>
`dropdown-item` here doesn't entirely work, and possibly other Bootstrap
classes either don't work as expected or are missing. Some related odd behavior
of language link/label:
- not logged in, no compact mode: no focus outline
- not logged in, compact mode: same + keyboard up/down controls stop working
correctly
- logged in, no compact mode: focus outline is present but non-standard because
no `nav-link`, but if you add it you'll need more css fixes
- logged in, compact mode: keyboard controls also not working correctly, in a
slightly different manner
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5201#pullrequestreview-2322665130
You are receiving this because you are subscribed to this thread.
Message ID:
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Add social sharing functionality (PR #4985)
@AntonKhorev commented on this pull request.
> @@ -0,0 +1,27 @@
+// Opening pop-ups with share URL
+function openShareUrl(url, initialWidth = 640, initialHeight = 480) {
+ if (typeof url !== "string" || !url.startsWith("http")) {
+console.error("Invalid URL"); // Consider removing this line if console
warnings should be avoided.
+return;
+ }
+
+ const width = Math.max(100, Math.min(screen.width, initialWidth));
+ const height = Math.max(100, Math.min(screen.height, initialHeight));
+
+ const left = (screen.width / 2) - (width / 2);
+ const top = (screen.height * 0.3) - (height / 2);
+ const opts =
`width=${width},height=${height},left=${left},top=${top},menubar=no,status=no,location=no`;
+
+ window.open(url, "popup", opts);
Still opens a window for `mailto:` links. What do you expect to appear in that
window?
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4985#discussion_r1769565165
You are receiving this because you are subscribed to this thread.
Message ID:
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Fixed displacing points specified from context menu (PR #4904)
@AntonKhorev commented on this pull request.
> -geocodeCallback();
- });
+ if (endpoint.latlng.distanceTo(L.latLng(json.lat, json.lon)) >
200.0) {
+input.val(endpoint.value);
+ } else {
+endpoint.value = json.display_name;
+input.val(json.display_name);
+ }
+
+ geocodeCallback();
+});
+ } else {
+$.getJSON(OSM.NOMINATIM_URL + "search?q=" +
encodeURIComponent(endpoint.value) + "&format=json&viewbox=" + viewbox,
function (json) {
+ endpoint.awaitingGeocode = false;
+ endpoint.hasGeocode = true;
+ if (!json || json.length === 0) {
Looked at what is already in use by included libraries:
- `let/const` - used by Bootstrap and Turbo
- `?.` - used by Turbo
- template literals - used by Bootstrap and Turbo
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4904#discussion_r1769576577
You are receiving this because you are subscribed to this thread.
Message ID:
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Browse icons as svg (PR #5080)
Some icons before/after in dark mode:   ;   ;   - some icons are too dark, I inverted and hue-rotated them; here it's not done I suppose; see place_of_worship icon for an extreme example - gate icon has white background - do we need a smaller icon for waste baskets here just because it's smaller on the map render? -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5080#issuecomment-2365214478 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Browse icons as svg (PR #5080)
I still think it's [easier](https://github.com/openstreetmap/openstreetmap-website/pull/5080#issuecomment-2298987792) to switch to ``s first, then start replacing them with svgs, assuming all of the images have the same size. Hopefully we can avoid random pixel offsets in css. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5080#issuecomment-2365223452 You are receiving this because you are subscribed to this thread. Message ID: ___ rails-dev mailing list rails-dev@openstreetmap.org https://lists.openstreetmap.org/listinfo/rails-dev
Re: [openstreetmap/openstreetmap-website] Browse icons as svg (PR #5080)
@AntonKhorev commented on this pull request.
> @@ -945,6 +945,76 @@ img.trace_image {
.node, .way, .relation {
margin-left: 25px;
}
+
+ .svg_icon {
+overflow: hidden;
+display: inline-block;
+margin-left: -25px;
+width: 25px;
+height: 18px;
+/*rtl:ignore*/ transform: translate(2px, 0px);
What's this `translate` for?
Old css background image above, new svg image below:
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5080#pullrequestreview-2319829810
You are receiving this because you are subscribed to this thread.
Message ID:
___
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev
