Re: [openstreetmap/openstreetmap-website] Add 'canDownloadImage' layer option (PR #5416)
@AntonKhorev pushed 2 commits. 7c15c9b0bab0ae64f00f71f2cc4d7c9bf52a0f55 Add 'canDownloadImage' layer option 276520135db18ed57524669dd03b4ec06778f827 Change image download warning message to a list -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5416/files/d74decfb409e5be88d2f23378dca03d990843963..276520135db18ed57524669dd03b4ec06778f827 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 'canDownloadImage' layer option (PR #5416)
Follows #5388. Removes hardcoded layer id checks in *Share > Image*, except for one special mapnik check. If a layer that can't be downloaded is selected, the message shows a list of available layers: Before:  After:  You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5416 -- Commit Summary -- * Add 'canDownloadImage' layer option * Change image download warning message to a list -- File Changes -- M app/assets/javascripts/leaflet.share.js (33) M config/layers.yml (3) M config/locales/en.yml (2) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5416.patch https://github.com/openstreetmap/openstreetmap-website/pull/5416.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5416 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] Refactor render_social_share_buttons helper (Issue #5415)
We have enough unused code in helpers. For example `auth_button` has `options` argument. Options for what? For link or image? Turns out it's for path. But it's never used. `user_image_url` also has `options`, also unused, and if you passed it, it would be used only in one branch. However all of that is insignificant compared to *classic pagination* #5205. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5415#issuecomment-2551885212 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] Refactor social sharing helper (PR #5417)
Are you sure you escape things correctly?  -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5417#issuecomment-2551977312 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 profile heatmap visualization for contributions (PR #5402)
I've tested the performance of the underlying query and using the user with the most changesets in the last year (just over 50 thousand) it took a fraction over one second which is probably acceptable. That result (which was a full 365 days of data) serialised to just under 10Kb which is a fair chunk to be caching but hopefully most entries will be smaller than that. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5402#issuecomment-2551259545 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 map tiles dark mode without leaflet-osm plugin (PR #5397)
@tomhughes commented on this pull request. This has gone rather further than I intended from my comments on #5396 by moving everything back into the web site code. My thinking was the leaflet-osm would still have the `darkUrl` attribute on the layer, and probably an option to the constructor that would cause it to be used if available, and only if that wasn't possible would the web site code activate a filter instead. The only thing I'm not sure about is how to signal back to this code whether dark more was possible. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5397#pullrequestreview-2511717176 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 map tiles dark mode without leaflet-osm plugin (PR #5397)
@tomhughes This is also further than I anticipated it would go. But I found it too hacky to overwrite the class's properties from the instance each time, and I especially wanted to avoid using `__proto__`. Yet I couldn't find a way to do that. But then I stumbled upon #5386 and its new layer definition handler that allowed me to eliminate those hacks since this would specify the scheme on the instance creation. Also not involving leaflet-osm at all would increase the flexibility with things like SVG filters, where I don't see an easy option to have that sourced from JS nicely. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5397#issuecomment-2551299817 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)
Merged #4985 into master. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/4985#event-15703610802 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.
> @@ -75,4 +76,32 @@ def render_flash(flash)
rescue StandardError
flash.inspect if Rails.env.development?
end
+
+ # Generates a set of social share buttons based on the specified options.
+ def render_social_share_buttons(opts = {})
+sites = opts.fetch(:allow_sites, [])
We can probably use it if sharing on some site requires some setting, to filter
out that site if it's not set, like if we add Mastodon host preference.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4985#discussion_r1890302583
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)
@tomhughes commented on this pull request.
> @@ -75,4 +76,32 @@ def render_flash(flash)
rescue StandardError
flash.inspect if Rails.env.development?
end
+
+ # Generates a set of social share buttons based on the specified options.
+ def render_social_share_buttons(opts = {})
+sites = opts.fetch(:allow_sites, [])
Well obviously we can come up with ways to use it, like adding a configuration
option to control which ones was enabled. But given that no such thing
currently exists I was wondering we it had been implemented - is there some
plan to make use of it in the future?
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4985#discussion_r1890313091
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] Move admin users list to its own controller (PR #5391)
Merged #5391 into master. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5391#event-15704036203 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] Center share popup inside the browser window (PR #5418)
What the existing share popup window placement does is it places the popup horizontally in the center of the screen. I don't know if the popup can get placed literally in the corner (https://github.com/openstreetmap/openstreetmap-website/issues/5414#issuecomment-2551615977), maybe with a multi-monitor setup? But it still could be a surprising location if the monitor is large enough. Here the logic is changed to place the popup in the center of the browser window. Is it less surprising? The only alternative to a popup window is a new tab, which is already available with middle click or context menu on share links. You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5418 -- Commit Summary -- * Center share popup inside the browser window -- File Changes -- M app/assets/javascripts/social_share_button.js (4) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5418.patch https://github.com/openstreetmap/openstreetmap-website/pull/5418.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5418 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 profile heatmap visualization for contributions (PR #5402)
> Oh, I forgot about this. But the user can now set his "Preferred Website > Color Scheme" which should be the basis of the theme not the browser setting. Thank you for the suggestion. I’ll refactor accordingly to ensure alignment with the preferred color scheme setting. > It is a very minor nitpick, but is it possible to dynamically determine > what's the first weekday based on the user's locale and adjust the heatmap > accordingly? > [Intl.Locale.prototype.getWeekInfo()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/Locale/getWeekInfo) > could be used. It isn't currently available in Firefox but all other > browsers have it. > > P.S. Added [bug > 1937867](https://bugzilla.mozilla.org/show_bug.cgi?id=1937867) to Firefox's > bug tracker. I tried dynamically adjusting the start and end weekdays using [ghDay](https://cal-heatmap.com/docs/options/subDomain#ghday) as the subdomain, but it rounds to the first and last weeks of the month, making implementation tricky. Accommodating this would require significant effort, which may not be worth it for such a minor adjustment. > What's the source(s) of all the javascript in `vendor/`? I would expect to > see `Vendorfile` updated, or preferably if these are sourced from e.g. node > modules, then they should be included in package.json and added using the > assets pipeline. If any of these are source files, i.e. written just for this > project, then they should be in app/. To be honest I wasn’t too familiar with how our assets pipeline works,so I just added the `cal-heatmap` library and its dependencies in `vendor/` and added required code in `manifest.js` and `application.js` to make it load correctly. If using `package.json` is preferred, I can update the PR accordingly. I'm not sure if there is some additional config needed for this to be accomplished or would updating the `package.json` file would be enough. Thank you for pointing out the existing `popper` inclusion. I’ll remove the duplicate entry from `application.js` and the `vendor` folder. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5402#issuecomment-2552523845 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] Alternatives to popup windows for social sharing (Issue #5414)
Anyone who wants new tabs can already open the links in new tabs by middle-clicking. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5414#issuecomment-2552540697 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] Center share popup inside the browser window (PR #5418)
@AntonKhorev pushed 1 commit. 2c57c66195a2c3a280fb33bf0a8f433d20650516 Center share popup inside the browser window -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5418/files/3fc487a08918fcdab1ec1c10c36e9cf94146c050..2c57c66195a2c3a280fb33bf0a8f433d20650516 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] Refactor social sharing helper (PR #5417)
This implements the refactorings suggested in #5415 and also improves the testing of the generated HTML to ensure that all the buttons are generated and reference the correct URL and title. You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5417 -- Commit Summary -- * Drop render prefix from social_share_buttons function * Drop unused ability to filter social sharing sites * Drop unused options from social_share_buttons * Use safe_join to join social sharing buttons * Improve testing of social sharing buttons -- File Changes -- M app/helpers/social_share_button_helper.rb (58) M app/views/diary_entries/show.html.erb (5) M test/helpers/social_share_button_helper_test.rb (35) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5417.patch https://github.com/openstreetmap/openstreetmap-website/pull/5417.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5417 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] Pass correct URL to Facebook sharing (PR #5412)
Merged #5412 into master. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5412#event-15704748998 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] Alternatives to popup windows for social sharing (Issue #5414)
I think that using a popup window for the social sharing functionality isn't a great user experience, but I would like to have a more detailed discussion about the pros and cons. In my mind, either one of these would be preferable: * Links open in a new tab, or * Use a [bootstrap modal dialog](https://getbootstrap.com/docs/5.3/components/modal/) What do other people think? Why were popups originally chosen? cc @kcne -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5414 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] Alternatives to popup windows for social sharing (Issue #5414)
> I think that using a popup window for the social sharing functionality isn't > a great user experience I certainly agree with that - in my case it was opening in the opposite corner of my monitor to the main window. Plus popups are pretty rare on the web these days so it will just surprise people in general I think. So I'm certainly interested in what the alternatives are. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5414#issuecomment-2551615977 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 'canEmbed' layer option (PR #5388)
There was something else, updated now. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5388#issuecomment-2551652330 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] Alternatives to popup windows for social sharing (Issue #5414)
> Links open in a new tab That's what already happens on small screens. If you do Bootstrap modal, it probably won't. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5414#issuecomment-2551596365 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 Bluesky to social sharing buttons (PR #5411)
Merged #5411 into master. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5411#event-15704766492 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] Pass correct URL to Facebook sharing (PR #5412)
Merged thanks. Feels to me like some tests are missing from the original PR, which might have caught this. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5412#issuecomment-2551563781 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] Move admin users list to its own controller (PR #5391)
Looks good to me, thanks. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5391#issuecomment-2551435630 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] Convert social share helper library into a real helper (PR #5413)
This converts the confusingly named `SocialShareButtonHelper` which is really a library, but tested by a helper test, into a real helper and moves the actual `render_social_share_buttons` helper there from the application helper. The other methods are then private as they're only used by `render_social_share_buttons` so are no longer tested separately as they're not visible to the test but will be tested implicitly by the calls to `render_social_share_buttons` in the other tests so coverage should be the same. You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5413 -- Commit Summary -- * Convert social share helper library into a real helper -- File Changes -- M app/helpers/application_helper.rb (29) R app/helpers/social_share_button_helper.rb (38) M test/helpers/social_share_button_helper_test.rb (12) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5413.patch https://github.com/openstreetmap/openstreetmap-website/pull/5413.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5413 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 map tiles dark mode without leaflet-osm plugin (PR #5397)
Given my limited familiarity with this repo, I decided to place cornerstones in the space of possibilities I saw rather than try for a perfect solution the first few times without much feedback. But now that I see a more modular solution I'll put this PR on hold too, just fixing the missing filter propagation, and move on to attempt 3 to avoid cluttering up the history. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5397#issuecomment-2551571726 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 map tiles dark mode without leaflet-osm plugin (PR #5397)
@hlfan pushed 1 commit. e77f38b1f2c0eaf7be817563268bcca14eb34569 Propagate layer filter to property -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5397/files/88c90f37fc7cc0dd3d1f0975565c7e75f392f155..e77f38b1f2c0eaf7be817563268bcca14eb34569 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] Convert social share helper library into a real helper (PR #5413)
Merged, thanks. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5413#issuecomment-2551593371 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] Dark mode preference not respected after OAuth expiry (Issue #5407)
I think this is unrelated to OAuth. I belive it's another way of asking for non-db preferences, so I'm closing this in favour of #5324 -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5407#issuecomment-2551569005 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 Bluesky to social sharing buttons (PR #5411)
Merged, thanks -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5411#issuecomment-2551565358 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 'canEmbed' layer option (PR #5388)
This seems reasonable to me, but it's marked as a draft. Is there something else that needs working on first, or is it just the merge conflicts now? -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5388#issuecomment-2551501150 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)
> I think you mean users who are not logged in yes > As for preferences, I'm happy keeping most preferences available for only > logged in users, since account creation is free and we aim to serve our > mappers (who by definition have accounts) more than just casual visitors. > Storing preferences in cookies does have drawbacks (like cross-device > consistency, automatic cookie expiry etc) so it's not my first choice in > general. Are you against the entire idea of #1115 or just against the cookies and want preferences stored somewhere else like session? I don't think anyone who is not logged in expects cross-device consistency etc. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/4985#issuecomment-2551576536 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] Convert social share helper library into a real helper (PR #5413)
Merged #5413 into master. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5413#event-15704922466 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] Refactor render_social_share_buttons helper (Issue #5415)
The implementation of render_social_share_buttons can be refactored in a number of ways: * `html_safe` **must not be used**. It does not convert strings to a safe representation, it is an assertion that the string is always safe given every possible input. This method is probably too complex for us to assert this today, and certainly not to ensure that everything involved is perfectly safe when features are added or removed from the helper in future. See e.g. `opengraph_tags` helper for the use of `safe_join` to combine tags in loop. * No need for a `render_` prefix in helper methods. See e.g. `opengraph_tags` helper * There are only two `opts` ever used in the method, `:title` and `:url`. Since there are only two, it would be better to call the method with these explicitly, ideally as keyword arguments so that the order doesn't matter. * The `filter_allowed_sites` method behaves surprisingly. If you pass it some sites, it will only return a subset of those sites. However, if you pass an empty set, instead of an empty list you instead get a full list of sites that were not requested. However, it's probably unlikely to be important, because... * The `filter_allowed_sites` method is unnecessary and unused code. It should be removed, since unnecessary and used code is both a source of bugs, and also makes it harder for new developers in future, who need to read and understand it. Personally I've spent time trying to review how it works and what it's trying to do, before realising that it's not used anywhere. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5415 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 'canEmbed' layer option (PR #5388)
@AntonKhorev pushed 3 commits. de0bf2549978f0b517c06c3eadaa477b37433ea6 Rename return values of getMapBaseLayerId() to layerId 2c84465cc2c131044cb290d596513723d4ce445a Add map.getMapBaseLayer() 8a8c2517f2e594bcecba1fda751ffd34b39dbe9a Add 'canEmbed' layer option -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5388/files/959a28ad11e5938136c0ee7fe345eb3ecce1d454..8a8c2517f2e594bcecba1fda751ffd34b39dbe9a 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 'canEmbed' layer option (PR #5388)
Merged, thanks! -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5388#issuecomment-2551713385 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 date and url settings for offline message (PR #5403)
@AntonKhorev pushed 7 commits. 5751f8e086035b30a96dc2ccd5b54c9a15e1abde Move map offline flash to partial d6afe1e4d14ad4e642ea3454545740a3294afab7 Use offline flash partial on edit page 4de0815b541a36b5cfe77d80f22c0800a4838433 Use offline flash partial on offline redirect target page 33eaeb800ef4dbe388abf317782217934f3dab2b Change scope of offline flash messages 9810d2ed9647a8dee9ea092355171b11aca264ee Add offline announcement setting 4242dbaf97616fdacd68a90c39f2336e5d147b43 Add expected restoration date setting 6569215f8b3b2d1dc1abfe013084bd7392687d51 Remove references to database maintenance from offline messages -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5403/files/337af7b299f49bac56f9d924e10acda600fba534..6569215f8b3b2d1dc1abfe013084bd7392687d51 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 'canEmbed' layer option (PR #5388)
Merged #5388 into master. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5388#event-15705677103 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 date and url settings for offline message (PR #5403)
Removed "database" from "maintenance":   -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5403#issuecomment-2551723793 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 standard patch/put api message update action (PR #5410)
Looks good to me, thanks. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5410#issuecomment-2551213402 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 Bluesky to social sharing buttons (PR #5411)
Adds Bluesky as an option in the social sharing buttons. You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5411 -- Commit Summary -- * Add Bluesky to social sharing buttons -- File Changes -- A app/assets/images/social_icons/bluesky.svg (5) M config/locales/en.yml (3) M lib/social_share_button_helper.rb (3) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5411.patch https://github.com/openstreetmap/openstreetmap-website/pull/5411.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5411 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 map tiles dark mode without leaflet-osm plugin (PR #5397)
> But I found it too hacky to overwrite the class's properties from the > instance each time, and I especially wanted to avoid using `__proto__`. Yet I > couldn't find a way to do that. I don't see any reason why what I was suggesting would need any of that but I've actually come up with a better solution now. What really bothered me was having the URL sometimes in leaflet-osm and sometimes here, so what I've done is to add a new `TransportDarkMap` layer to leaflet-osm in https://github.com/openstreetmap/leaflet-osm/commit/ebfaf22426c265aaf3f401d70ffd02ca920623c9. So now we can have a `leafletOsmDarkId: "TransportDarkMap"` key in `layers.yml` and if that key exists we construct that layer instead of the normal one and then don't do any filtering. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5397#issuecomment-2551365251 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)
Merged, thanks. Although we're effectively outsourcing Mastodon host option to mastodonshare.com, it works for anonymous users too. Currently our preferences page is not available for anonymous users but we probably should make it available and save preferences to cookies. We have other preferences like #5201 and color mode settings and we shouldn't add a button for each one. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/4985#issuecomment-2551409391 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 standard patch/put api message update action (PR #5410)
Merged #5410 into master. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5410#event-15703693652 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)
@tomhughes commented on this pull request.
> @@ -75,4 +76,32 @@ def render_flash(flash)
rescue StandardError
flash.inspect if Rails.env.development?
end
+
+ # Generates a set of social share buttons based on the specified options.
+ def render_social_share_buttons(opts = {})
+sites = opts.fetch(:allow_sites, [])
What is the purpose of the `allow_sites` option and the supporting logic that
allows filtering? As far as I can tell it's tested but never actually used by
any other code?
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4985#pullrequestreview-2511921655
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] Dark mode preference not respected after OAuth expiry (Issue #5407)
Closed #5407 as completed. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5407#event-15704788689 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] Pass correct URL to Facebook sharing (PR #5412)
It was trying to pass the literal text `params[:url]` as the URL due to incorrect quoting. You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5412 -- Commit Summary -- * Pass correct URL to Facebook sharing -- File Changes -- M lib/social_share_button_helper.rb (2) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5412.patch https://github.com/openstreetmap/openstreetmap-website/pull/5412.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5412 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)
> anonymous users I think you mean users who are not logged in - "anonymous users" is the term we use for accounts with `user.data_public == false`. They are quite rare now since [they haven't been permitted for a long time](https://wiki.openstreetmap.org/wiki/Anonymous_edits). As for preferences, I'm happy keeping most preferences available for only logged in users, since account creation is free and we aim to serve our mappers (who by definition have accounts) more than just casual visitors. Storing preferences in cookies does have drawbacks (like cross-device consistency, automatic cookie expiry etc) so it's not my first choice in general. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/4985#issuecomment-2551529816 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 date and url settings for offline message (PR #5403)
> Maybe we should just remove the references to database maintenance in favour > of this solution with a URL to an explanatory notice? Perhaps this is generic enough: ``` The OpenStreetMap database is currently in read-only mode while essential maintenance work is carried out. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5403#issuecomment-2551069076 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 map tiles dark mode without leaflet-osm plugin (PR #5397)
@gravitystorm requested changes on this pull request.
I'm very happy to see this work, thanks @hlfan!
The code looks fine to me, but I'm not a javascript expert so maybe there are
better ways to write parts.
Two other changes requested:
* Please rework your commits (e.g. using `git rebase -i`) to avoid having the
fixup commits (e.g. combine the linting fixes into the original commits)
* There's still a darken filter applied to the map previews - you can see that
the colour of the green spaces on the transport dark map are darkened compared
to the main map
> @@ -52,9 +57,19 @@ L.OSM.Map = L.Map.extend({
code: "G"
});
-this.on("layeradd", function (event) {
- if (this.baseLayers.indexOf(event.layer) >= 0) {
-this.setMaxZoom(event.layer.options.maxZoom);
+this.on("layeradd", function ({ layer }) {
+ if (this.baseLayers.indexOf(layer) >= 0) {
+this.setMaxZoom(layer.options.maxZoom);
+const container = layer.getContainer();
+if (!container) return;
+if (layer.options.schemeClass)
container.classList.add(layer.options.schemeClass);
+const filterRecievers = [container];
minor spelling error - `filterReceivers`
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5397#pullrequestreview-2511562149
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] Improve note testing and text handling (PR #5419)
- Add test that would have failed in #5359.
- Change the code that selects the text input from `$("textarea")` to
`content.find("textarea")`. This should stop the html share code from
getting posted to notes, although I couldn't reproduce that happening like
@SomeoneElseOSM reported it yesterday. When you press *Hide* the textarea
inside the left sidebar exists and is the first one on the page, I don't
know how that other textarea got selected instead.
You can view, comment on, or merge this pull request online at:
https://github.com/openstreetmap/openstreetmap-website/pull/5419
-- Commit Summary --
* Test share textarea not to leak into note text on reactivation
* Get note text only from textarea within left sidebar
* Move blocked resolve/reactivate tests to ResolveNoteTest
* Use within_sidebar in CreateNoteTest
-- File Changes --
M app/assets/javascripts/index/note.js (2)
M test/system/create_note_test.rb (16)
M test/system/note_comments_test.rb (46)
A test/system/resolve_note_test.rb (106)
-- Patch Links --
https://github.com/openstreetmap/openstreetmap-website/pull/5419.patch
https://github.com/openstreetmap/openstreetmap-website/pull/5419.diff
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5419
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] Alternatives to popup windows for social sharing (Issue #5414)
Popups were chosen initially, inspired by [rails-social-share-button](https://github.com/Bunlong/rails-social-share-button), which is now deprecated. I didn't thought too much about alternatives at the time. To be honest, I’m not a fan of popups either. Refactoring to open links in new tabs would probably simplify the code and make maintenance easier. For the other alternatives, using Bootstrap modals with iframes likely won’t work due to CSP restrictions on most websites. While the [Web Share API](https://developer.mozilla.org/en-US/docs/Web/API/Web_Share_API#browser_compatibility) is promising, it’s not yet widely supported. Opening links in new tabs seems like the cleanest and most maintainable solution in my opinion. I would be eager to do a refactor if there is agreement on the approach. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5414#issuecomment-2552426298 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] Refactor render_social_share_buttons helper (Issue #5415)
First of all, thank you for taking the time to do this post-merge review @gravitystorm . I wish these points had been raised earlier so I could have reassessed and addressed them before merging. I completely agree with your observations. As mentioned earlier, I drew some inspiration from the rails library mentioned in the issue. The `filter_allowed_sites` method was intended to validate a given list of sites and render only the valid ones. If no list was provided, it would default to rendering all sites, assuming that social share buttons without any provider wouldn’t be practical. That said, I agree the implementation could have been cleaner and better aligned with our use case. I noticed that @tomhughes has already opened #5417 to address this. I wish i was active at the time issue was raised to do it instead. Never the less, I appreciate the feedback and will keep this in mind as we refactor and finalize social sharing functionality. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5415#issuecomment-2552446197 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] Refactor social sharing helper (PR #5417)
@kcne approved this pull request. Everything works well on my side. Thank you. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5417#pullrequestreview-2513042514 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] Refactor social sharing helper (PR #5417)
> Are you sure you escape things correctly? >  This only happens on [Mastodon Share](https://mastodonshare.com/) links. Must be something related to their implementation. I'm not sure if the code is opensource since i could not find the github repo. On the web site about page there is only this information: > MastodonShare is 100% private, 100% free, and 100% secure. It does not store > or track anything about the visitors, usernames, instances, links or content. > It does not require Mastodon API access. > If you have any questions, please ask me > [a...@barredo.es](mailto:a...@barredo.es) or > [@barredo@mastodon.social](https://mastodon.social/@barredo). -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5417#issuecomment-2552464775 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] Refactor social sharing helper (PR #5417)
Looks like `&` gets unescaped in mastodonshare.com's redirect location. Request headers: ``` GET /?text=A%26B&url=http%3A%2F%2F127.0.0.1%3A3000%2Fuser%2Ffakeuser1%2Fdiary%2F86 HTTP/1.1 Host: mastodonshare.com ... ``` Response headers: ``` HTTP/1.1 302 Found ... Location: https://en.osm.town/share?text=A&B%0A%0Ahttp://127.0.0.1:3000/user/fakeuser1/diary/86 ``` -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5417#issuecomment-2552483990 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
