Re: [openstreetmap/openstreetmap-website] Don't classify Coveralls upload server errors as failures (Issue #5670)
hlfan left a comment (openstreetmap/openstreetmap-website#5670) I've been thinking about if more granular control about which errors get reported in which way could help. (maybe issue at [coverallsapp/github-action](https://github.com/coverallsapp/github-action)?) -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5670#issuecomment-2764572224 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 classify Coveralls upload server errors as failures (Issue #5670)
tomhughes left a comment (openstreetmap/openstreetmap-website#5670) > we can always change this temporarily if there is a problem with coveralls > that we need to work around. This is not really true though as once it starts happening changing the workflow won't have any effect on existing PRs unless they are rebased. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5670#issuecomment-2764574359 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 classify Coveralls upload server errors as failures (Issue #5670)
tomhughes left a comment (openstreetmap/openstreetmap-website#5670) > I've been thinking about if more granular control about which errors get > reported in which way could help. (maybe issue at > [coverallsapp/github-action](https://github.com/coverallsapp/github-action)?) I was just looking at what's possible and the action has a `fail-on-error` setting but that basically makes pretty much any sort of error not an error though it's far from clear to me what errors you would want to exclude to cover the cases @gravitystorm seems to be worried about. Can you ever distinguish between an HTTP error that means the API is down and one that means it rejected your data because it was the wrong format? At least without intimate knowledge of the server side... -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5670#issuecomment-2764575427 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] Rephrase strings with the word "followings" (PR #5857)
@hlfan approved this pull request. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5857#pullrequestreview-2727853929 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] Fix GitHub badges in readme (PR #5849)
AntonKhorev left a comment (openstreetmap/openstreetmap-website#5849) > Alternative after ... #5670 this pr had nothing to do with Coveralls -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5849#issuecomment-2764630283 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 SVG symbol tags (PR #5776)
@hlfan pushed 1 commit. 2d9a37a61e1dabf65bed726f0a9f7df62877be91 Remove svg symbol tags -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5776/files/0f7b954def5bf58c24dba964d8825a3bb08c7e04..2d9a37a61e1dabf65bed726f0a9f7df62877be91 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] Rephrase strings with the word "followings" (PR #5857)
@tomhughes approved this pull request. Thanks for the fix. I think this looks good now. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5857#pullrequestreview-2727906151 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 differently sized direction icon arrows with a marker (PR #5866)
Merged #5866 into master. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5866#event-17047999638 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] Fix GitHub badges in readme (PR #5849)
hlfan left a comment (openstreetmap/openstreetmap-website#5849) Alternative after:  #5670 -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5849#issuecomment-2764620206 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] Propagate iD title to parent (PR #5865)
@AntonKhorev commented on this pull request.
> @@ -76,5 +76,13 @@ document.addEventListener("DOMContentLoaded", function () {
const data = parent.OSM.mapParams();
goToLocation(data);
});
+
+const firstParentTitle = parent.document.title;
+new MutationObserver(function (mutations) {
+ if (!mutations.some(mutation => mutation.target.tagName === "TITLE"))
return;
+ const newTitle = `${document.title} | ${firstParentTitle}`;
+ if (parent.document.title === newTitle) return;
+ parent.document.title = newTitle;
+}).observe(document.head, { childList: true, subtree: true });
Maybe also watch for `characterData`. Currently there's no difference because
the title is changed by deleting the text node and creating a new one, but its
going to be undetected if done by a node value change. I don't know how likely
it's for iD to do that.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5865#pullrequestreview-2727975738
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] Propagate iD title to parent (PR #5865)
@AntonKhorev commented on this pull request.
> + if (!mutations.some(mutation => mutation.target.tagName === "TITLE"))
> return;
+ const newTitle = `${document.title} | ${firstParentTitle}`;
+ if (parent.document.title === newTitle) return;
+ parent.document.title = newTitle;
```suggestion
parent.document.title = `${document.title} | ${firstParentTitle}`;
```
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5865#pullrequestreview-2727973616
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 classify Coveralls upload server errors as failures (Issue #5670)
tomhughes left a comment (openstreetmap/openstreetmap-website#5670) This is the current coveralls status, and the end time has already been extended at least once:  Coveralls posts it's results separately in the actions results, so do we really need to have it also show as a hard test failure? -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5670#issuecomment-2764569367 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] Propagate iD title to parent (PR #5865)
@AntonKhorev commented on this pull request.
> @@ -76,5 +76,13 @@ document.addEventListener("DOMContentLoaded", function () {
const data = parent.OSM.mapParams();
goToLocation(data);
});
+
+const firstParentTitle = parent.document.title;
+new MutationObserver(function (mutations) {
+ if (!mutations.some(mutation => mutation.target.tagName === "TITLE"))
return;
+ const newTitle = `${document.title} | ${firstParentTitle}`;
+ if (parent.document.title === newTitle) return;
+ parent.document.title = newTitle;
+}).observe(document.head, { childList: true, subtree: true });
You have to observe `document.head` because there's no title element initially.
However if you `` to `app/views/site/id.html.erb`, you'll be able to
observe it right away. Even an empty title should work.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5865#discussion_r2020224432
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 classify Coveralls upload server errors as failures (Issue #5670)
tomhughes left a comment (openstreetmap/openstreetmap-website#5670) I really think we need to revisit this - having every PR show as erroneous because coveralls is down for maintenance for long periods is a massive pain for maintainers trying to work out what can be reviewed. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5670#issuecomment-2764568748 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] Revert "Replace differently sized arrows with a marker" (PR #5867)
tomhughes left a comment (openstreetmap/openstreetmap-website#5867) Sorry yes I just realised I merged this by accident - it wasn't what I intended to merge. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5867#issuecomment-2764669124 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] Rephrase strings with the word "followings" (PR #5857)
Merged #5857 into master. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5857#event-17048553887 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 differently sized direction icon arrows with a marker (PR #5866)
tomhughes left a comment (openstreetmap/openstreetmap-website#5866) Sorry @hlfan I merged this accidentally and have now reverted it but there doesn't seem to be any way to reopen the PR so if you want to pursue it can you reopen but you might want to read @AntonKhorev's comments in #5867 first. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5866#issuecomment-2764670872 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] Revert "Replace differently sized arrows with a marker" (PR #5867)
Merged #5867 into master. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5867#event-17048553883 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 OpenMapTiles vector map (PR #4042)
@hlfan commented on this pull request. > +<% if Settings.key?(:maptiler_key) %> + MAPTILER_KEY: <%= Settings.maptiler_key.to_json %>, +<% end %> This should be with the other keys in the embedded Settings json -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/4042#pullrequestreview-2728168639 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] Propagate iD title to parent (PR #5865)
@hlfan commented on this pull request.
> @@ -76,5 +76,10 @@ document.addEventListener("DOMContentLoaded", function () {
const data = parent.OSM.mapParams();
goToLocation(data);
});
+
+const projectTitle = parent.document.title;
+new MutationObserver(() =>
+ parent.document.title = [document.title, projectTitle].filter(t =>
t).join(" | ")
+).observe(document.querySelector("title"), { childList: true,
characterData: true });
Took out `subtree` while moving to watch the title directly and forgot to put
it back in.
_"Sometimes, my [confusion] is... It's almost frightening."_
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5865#discussion_r2020477550
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] Propagate iD title to parent (PR #5865)
@hlfan pushed 1 commit. 7a8bc77219d111cc9f78cff0e32ee787b03aea20 Propagate iD title to parent -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5865/files/e8ac8ffe3cdb4ebca7919d275ce5dc89f9a0aed5..7a8bc77219d111cc9f78cff0e32ee787b03aea20 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] Propagate iD title to parent (PR #5865)
@AntonKhorev commented on this pull request.
> @@ -76,5 +76,10 @@ document.addEventListener("DOMContentLoaded", function () {
const data = parent.OSM.mapParams();
goToLocation(data);
});
+
+const projectTitle = parent.document.title;
+new MutationObserver(() =>
+ parent.document.title = [document.title, projectTitle].filter(t =>
t).join(" | ")
+).observe(document.querySelector("title"), { childList: true,
characterData: true });
I don't know why you removed `subtree` and how `characterData` is going to work
without it.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5865#pullrequestreview-2728297441
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 SVG symbol tags (PR #5776)
@hlfan pushed 1 commit. dd4068c8c7638beeb8e39181ded99dfc82dec90f Remove svg symbol tags -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5776/files/2d9a37a61e1dabf65bed726f0a9f7df62877be91..dd4068c8c7638beeb8e39181ded99dfc82dec90f 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 SVG symbol tags (PR #5776)
@hlfan pushed 1 commit. 0f7b954def5bf58c24dba964d8825a3bb08c7e04 Remove svg symbol tags -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5776/files/dd4068c8c7638beeb8e39181ded99dfc82dec90f..0f7b954def5bf58c24dba964d8825a3bb08c7e04 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 differently sized direction icon arrows with a marker (PR #5866)
### Description The way the directions icons are currently drawn, the diagonal arrows are a fair bit smaller than the arrows in cardinal directions. The size stays consistent when the arrows are instead drawn with a marker. Compare the icons in this chart, where the old icon is red, the new one green, and the overlap yellow:  This requires the SVG element to be visible, so d-none was swapped. ### How has this been tested? in-browser You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5866 -- Commit Summary -- * Replace differently sized arrows with a marker -- File Changes -- M app/views/directions/search.html.erb (29) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5866.patch https://github.com/openstreetmap/openstreetmap-website/pull/5866.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5866 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] Propagate iD title to parent (PR #5865)
@hlfan pushed 1 commit. e8ac8ffe3cdb4ebca7919d275ce5dc89f9a0aed5 Propagate iD title to parent -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5865/files/5ff2d6a204f136158aabdb1bce3434e2145511df..e8ac8ffe3cdb4ebca7919d275ce5dc89f9a0aed5 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] Search and latlon/nominatim query resources (PR #5868)
@hlfan commented on this pull request.
> +const params = new URLSearchParams;
+for (const paramName of ["query", "zoom", "minlon", "minlat", "maxlon",
"maxlat"]) {
+ const paramValue = this.elements[paramName].value;
+ if (paramValue) {
+params.set(paramName, paramValue);
+ }
+}
+const search = params.get("query") ? `/search?${params}` : "/";
Do you really want to expose all these parameters in the URL?
The bbox params don't even update the map hash that's also there
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5868#pullrequestreview-2728284278
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 differently sized direction icon arrows with a marker (PR #5866)
AntonKhorev left a comment (openstreetmap/openstreetmap-website#5866) You want horizontal and vertical shape boundaries to go exactly over pixel boundaries, otherwise shapes may look more blurry and smaller. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5866#issuecomment-2764695781 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
