Re: [openstreetmap/openstreetmap-website] add dir="auto" to paragraphs in rich_text.rb (PR #5835)
NeatNit left a comment (openstreetmap/openstreetmap-website#5835) I would like to handle Markdown in this PR as well. Where on the OSM website is Markdown used? I'd like to check if it's already handled by default. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5835#issuecomment-2744665685 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 dir="auto" to paragraphs in rich_text.rb (PR #5835)
NeatNit left a comment (openstreetmap/openstreetmap-website#5835) Awesome, I'll do that ASAP :) -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5835#issuecomment-2746262433 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 dir="auto" to paragraphs in rich_text.rb (PR #5835)
NeatNit left a comment (openstreetmap/openstreetmap-website#5835) > In fact this is going to just wind up as a single commit I think so squash > everything together unless you have a good reason not to. That's what I intend to do. Just need to rebase with the new master. Will get behind a computer in a few minutes to do it. I contemplated doing it from termux, but that sounds too painful. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5835#issuecomment-2746266111 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 dir="auto" to paragraphs in rich_text.rb (PR #5835)
@NeatNit pushed 1 commit. 50c79d89503194c34f9a325c0aa53140a6d0b65e undo common.scss changes -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5835/files/83575bd9baee1e49d3e3d822d8fd01a423cb1596..50c79d89503194c34f9a325c0aa53140a6d0b65e 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 dir="auto" to Markdown content for bidi (PR #5840)
NeatNit left a comment (openstreetmap/openstreetmap-website#5840) > I wonder if this would this be better done with a sanitize transformer that > would work on the HTML regardless of how it was generated? I don't know. Is there anything wrong with this approach? It seems to be the cleanest way to do it within kramdown. In what way would sanitize be better? -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840#issuecomment-2746421822 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 dir="auto" to Markdown content for bidi (PR #5840)
@NeatNit pushed 1 commit. 618df632c8509e5367ae990494c7418465dd2439 make markdown bidirectional with dir="auto" -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840/files/d55aca5d61f7b90702ec161e3ce1860430e655f8..618df632c8509e5367ae990494c7418465dd2439 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] Set dir="auto" on changeset comment bodies (Issue #5785)
NeatNit left a comment (openstreetmap/openstreetmap-website#5785) @gravitystorm I've opened a pull request now, completely untested but I hope it gets the ball rolling. I honestly don't think it has much distance to roll. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5785#issuecomment-2744377461 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 dir="auto" to paragraphs of formatted text (PR #5835)
NeatNit left a comment (openstreetmap/openstreetmap-website#5835) > Something like this: How about almost exactly that :) I applied it to note comments as well. I'm not sure if any other places need the same treatment. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5835#issuecomment-2744580875 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 dir="auto" to paragraphs of formatted text (PR #5835)
@NeatNit pushed 1 commit. b4cae9b7a556cc093138fb728b8bced879d8d773 add text-align: start; to comment_text class (inherited individually by each descendant p) -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5835/files/31742c86b49e02483f0bdd01eb910797f1fa0386..b4cae9b7a556cc093138fb728b8bced879d8d773 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 dir="auto" to paragraphs of formatted text (PR #5835)
@NeatNit pushed 1 commit. d5b79d81a9ac632607e9484870611f0fd3979d78 add comment_text class to changeset comment div -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5835/files/148ec0a3656f884cc388b8259aa8ef1ec3aa3eb8..d5b79d81a9ac632607e9484870611f0fd3979d78 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 dir="auto" to paragraphs of formatted text (PR #5835)
@NeatNit pushed 1 commit. 31742c86b49e02483f0bdd01eb910797f1fa0386 add comment_text class to note comment div -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5835/files/d5b79d81a9ac632607e9484870611f0fd3979d78..31742c86b49e02483f0bdd01eb910797f1fa0386 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 dir=auto to user generated content (PR #3429)
NeatNit left a comment (openstreetmap/openstreetmap-website#3429) I know this hasn't seen any action in 3 years, but if this is ever picked up again you should know: My PR #5835 applies `dir="auto"` to each paragraph individually in rich text content. This produces better results, in line with every other platform used today (e.g. this very comment, or your favorite instant messaging app). It also makes some - though definitely not all - of the changes in this PR redundant. Non-rich text like usernames still needs this PR, but rich text that converts to HTML with `to_html` is better handled "upstream" by that conversion. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/3429#issuecomment-2744664490 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 dir="auto" to paragraphs of formatted text (PR #5835)
@NeatNit pushed 1 commit. 148ec0a3656f884cc388b8259aa8ef1ec3aa3eb8 lint fix -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5835/files/6c05f1e9c9757e4d65ec2bef116e5373af71804c..148ec0a3656f884cc388b8259aa8ef1ec3aa3eb8 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 dir="auto" to paragraphs of formatted text (PR #5835)
NeatNit left a comment (openstreetmap/openstreetmap-website#5835)
Related to #3432, it seems that in the case of changeset comments, they are
arranged in a list, and `text-align` is being set by the browser stylesheet (in
Firefox at least) to `` elements:
```css
li {
display: list-item;
text-align: match-parent;
}
```
This makes sense for lists in their usual setting but not so much in this case.
So one possible solution is:
```css
li p {
text-align: start;
}
```
So that paragraphs won't inherit the alignment from their parent too. But
again, I'm not a web developer, so more professional discretion might be needed.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5835#issuecomment-2744480698
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 dir="auto" to paragraphs of formatted text (PR #5835)
@NeatNit pushed 1 commit. 6c05f1e9c9757e4d65ec2bef116e5373af71804c move the simple_format extra argument to where it actually works -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5835/files/7a263e7cbcbfadc8be0a2797d2480542b2546bd5..6c05f1e9c9757e4d65ec2bef116e5373af71804c 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 dir="auto" to paragraphs of formatted text (PR #5835)
@NeatNit commented on this pull request. > @@ -86,7 +86,7 @@ def linkify(text, mode = :urls) class HTML < Base def to_html - linkify(sanitize(simple_format(self))) + linkify(sanitize(simple_format(self, dir: "auto"))) Yup, that's what I thought. How about this? (commit I just pushed) -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5835#discussion_r2008357190 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 dir="auto" to Markdown content for bidi (PR #5840)
NeatNit left a comment (openstreetmap/openstreetmap-website#5840) I guess I should have mentioned that I did improve handling of `` tags. Ready for your scathing reviews :P -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840#issuecomment-2749569794 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 dir="auto" to paragraphs in rich_text.rb (PR #5835)
NeatNit left a comment (openstreetmap/openstreetmap-website#5835) > Sidebar comments are in html lists, but should they? This is above my pay grade 😉 and definitely outside the scope of this PR -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5835#issuecomment-2745338312 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 dir="auto" to Markdown content for bidi (PR #5840)
### Description 1. Derive a kramdown converter subclass that adds `dir="auto"` to HTML elements where appropriate. 2. Call this new converter instead of the default HTML converter 3. Add `dir="auto"` to changeset comment tag (unrelated, should have been part of #5835) This solves the last (to my knowledge) of the most prominent user-generated content bidi issues of #3428, that being user diaries. Other less prominent issues are handled in #3429, which best I can tell does not overlap with this PR. ### How has this been tested? The kramdown subclass was tested here: https://codeberg.org/NeatNit/kramdown-bidi/src/branch/main/code.rb Its integration into the OpenStreetMap website **has not been tested.** I don't expect you'd like the amount of comments in that converter, or that github issue comment link in the source code. In my opinion it's needed to understand the choices I've made. If you prefer, I can remove some of the comments, just tell me which ones. You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5840 -- Commit Summary -- * add dir="auto" to changeset comment * make markdown bidirectional with dir="auto" -- File Changes -- M app/views/changesets/show.html.erb (2) M lib/rich_text.rb (42) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5840.patch https://github.com/openstreetmap/openstreetmap-website/pull/5840.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840 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 dir="auto" to Markdown content for bidi (PR #5840)
@NeatNit pushed 1 commit. 2a24d8abe8bf6b049dfe3e5726884a0f4eec0d5b make markdown bidirectional with dir="auto" -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840/files/618df632c8509e5367ae990494c7418465dd2439..2a24d8abe8bf6b049dfe3e5726884a0f4eec0d5b 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] Bidi support for texts of diaries and changeset discussions (#1943)
NeatNit left a comment (openstreetmap/openstreetmap-website#1943) FWIW, this has finally been fixed for discussions yesterday in my PR #5835 and may soon be fixed for diary entries by #5840 if accepted. This issue is more-or-less a duplicate of #3428. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/1943#issuecomment-2749565996 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] Set dir="auto" on changeset comment bodies (Issue #5785)
NeatNit left a comment (openstreetmap/openstreetmap-website#5785) > Duplicate of > [#3428](https://github.com/openstreetmap/openstreetmap-website/issues/3428) The very first sentence I wrote is that it's a sub-issue of that. My issue is basically an almost-PR, saying exactly how I think this can be solved for changeset comments specifically. The only reason it's not an actual PR is that I don't have the means to test these changes, nor the expensive with this programming language/environment to know whether I'm doing something dumb. If you want, I can make it an actual PR and make it super clear that it's untested and needs careful review. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5785#issuecomment-2740460529 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 dir="auto" to Markdown content for bidi (PR #5840)
NeatNit left a comment (openstreetmap/openstreetmap-website#5840) Ready for review. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840#issuecomment-2776243614 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 dir="auto" to Markdown content for bidi (PR #5840)
NeatNit left a comment (openstreetmap/openstreetmap-website#5840) I just realized I should make that smarter handling of `` tags before merging this. Marked as draft in the meantime. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840#issuecomment-2746448148 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 dir="auto" to Markdown content for bidi (PR #5840)
@NeatNit pushed 1 commit. fe6262fe0c4ab0f53c329477534c4bf08464ffc1 add dir="auto" to linkify links -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840/files/c171c8575e0b7a0061a9a1c577ada5c765a684ee..fe6262fe0c4ab0f53c329477534c4bf08464ffc1 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 dir="auto" to paragraphs of formatted text (PR #5835)
### Description This is my attempt to create a pull request for the changes I suggested in #5785. It should add the `dir="auto"` HTML property to user-generated content (such as changeset comments) that uses the classes in `lib/rich_text.rb`. I can only hope this would apply to all or most of the user-generated content, thus providing a solution for #3428, but only someone intimately familiar with this repository would be able to say. I also hope this applies *only* to user-generated content and not other parts of the website - however, it probably wouldn't cause any harm even if applied to other elements. I never user Ruby before (and technically, I still haven't... see "How has this been tested?"), so this change is entirely based on the documentation found [here](https://api.rubyonrails.org/classes/ActionView/Helpers/TextHelper.html#method-i-simple_format). I hope I've understood it correctly. **This PR is still incomplete:** it needs the CSS declaration `text-align: start;`, but I don't know which rule it would be best to apply it to (or even where CSS files are in this repository). Ideally it should affect any and all HTML elements on the website that contain user-written content. Advice on where to add that would be appreciated. That said, merging it even in this state without the CSS should still provide some benefit and no harm, assuming it works as I imagine. This is why I'm not marking it as draft. ### How has this been tested? **This has not been tested at all. For all I know, it might even contain syntax errors.** I don't currently have the means to clone and run this repository. I know this is bad form to put it mildly, but with a change this small and focused I am hoping you can forgive and review it nonetheless, and run any tests in your already-set-up environment. If this is totally unacceptable, let me know. You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/5835 -- Commit Summary -- * add dir="auto" to paragraphs of formatted text -- File Changes -- M lib/rich_text.rb (4) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/5835.patch https://github.com/openstreetmap/openstreetmap-website/pull/5835.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5835 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 dir="auto" to Markdown content for bidi (PR #5840)
@NeatNit pushed 1 commit. c171c8575e0b7a0061a9a1c577ada5c765a684ee make markdown bidirectional with dir="auto" -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840/files/440e1fe705ad630e55af8007b178f786e036c238..c171c8575e0b7a0061a9a1c577ada5c765a684ee 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 dir="auto" to paragraphs of formatted text (PR #5835)
NeatNit left a comment (openstreetmap/openstreetmap-website#5835) Trying to read the code more thoroughly (keep in mind it's still a bit alien to me), I'm fairly sure I'm adding that parameter in the wrong place... But uhh, it's the thought that counts, right? 😅 I really don't care if my PR gets merged or not, so if you want to just take the idea and implement it correctly in a separate PR, that's fine by me. It will probably be the most efficient way for everyone involved. Otherwise, I might *eventually* have the means to do this properly, but my timeline for that is really not good (optimistically I *might* be able to start trying it over a month from now). -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5835#issuecomment-2744424468 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 dir="auto" to Markdown content for bidi (PR #5840)
@NeatNit pushed 1 commit. d55aca5d61f7b90702ec161e3ce1860430e655f8 make markdown bidirectional with dir="auto" -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840/files/16168bb4d7bd1bdfbc984af1301a0900ae84806f..d55aca5d61f7b90702ec161e3ce1860430e655f8 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 dir="auto" to paragraphs in rich_text.rb (PR #5835)
NeatNit left a comment (openstreetmap/openstreetmap-website#5835) Yay! How long do you think until it goes live on osm.org? -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5835#issuecomment-2746295538 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 dir="auto" to Markdown content for bidi (PR #5840)
NeatNit left a comment (openstreetmap/openstreetmap-website#5840) In that case, I stand by the current version of this PR. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840#issuecomment-2751739808 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 dir="auto" to Markdown content for bidi (PR #5840)
@NeatNit commented on this pull request.
> +# "li", # child; sometimes container
+
+"table", # similar to Discourse, where the parent of
gets dir="auto" (no parent div in kramdown)
+# "td", # child
+# "th", # child; not actually a thing in kramdown but hypothetically
might be in the future
+
+"dl",
+# "dd", "dt", # child - since it's similar to a list, it has the same
behavior as a list
+
+"math" # don't know how this ends up, but dir="auto" is probably
correct
+ ].each do |name|
+define_method :"convert_#{name}" do |el, indent|
+ attr_bak = el.attr.dup # can't avoid mutating the attr hash, so make
a backup
+ el.attr["dir"] = "auto" unless el.attr.key?("dir") # if by some
miracle dir is already defined, don't override it
+ ret = super(el, indent)
+ el.attr.replace(attr_bak) # restore backup
> What happens if we don't restore the original attributes? Does the element
> get reused in a way that breaks things?
Truthfully, nothing. It's only a concern if you ever use a converter other than
HtmlBidi. Since we don't, it doesn't matter, and we can mutate the Document
freely.
> would just doing `el = el.dup` as the first thing to duplicate the element be
> a workable alternative?
No, because it would still point to the same `attr` hash, and mutating it would
mutate the original. But as I said, it doesn't matter, so the whole
backup-restore code can be removed if you wish.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5840#discussion_r2014856583
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 dir="auto" to Markdown content for bidi (PR #5840)
NeatNit left a comment (openstreetmap/openstreetmap-website#5840) > This is a version that modifies the document, consisting of replaced > `document` method, assuming we don't need to add `dir` on inline elements: > > ```ruby > def document > return @document if @document > > @document = Kramdown::Document.new(self) > > add_dir = lambda do |element| > target_types = %w[p header pre codeblock ul ol table dl math] > element.attr["dir"] ||= "auto" if target_types.include?(element.type > == :html_element ? element.value : element.type.to_s) > element.children.each(&add_dir) > end > add_dir.call(@document.root) > > @document > end > ``` Thank you, I want to use this method instead of mine. Just a couple of questions: Is it good practice to use a lambda like that? Wouldn't it be better to define a function normally and call that? (I'm a Ruby newbie, but this seems a bit strange to me) And, is `element.type == :html_element` supposed to catch elements written as HTML within the Markdown source? In which case I think we should not add `dir="auto"` - if the user crafts their own HTML for whatever reason, we shouldn't change their work. I will not be able to make modifications to the code for the next few days (until Tuesday or Wednesday, most likely), but I'd like to do this: - [ ] Modify document upon creation instead of deriving a converter (i.e. what I replied to just now) - [ ] Make `linkify` add `dir="ltr"` to links - this would also affect changeset comments and the like - [ ] Made triple sure that links made with `` markdown syntax actually get `dir="auto"` - I did not test this enough and it likely fails in some cases. Namely when the URL contains Unicode characters and they get replaced by `%D7%90` or whatever. I'll make these changes as soon as I can, next week. If any more feedbacks comes up I'll handle it too. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840#issuecomment-2759773367 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 dir="auto" to Markdown content for bidi (PR #5840)
@NeatNit commented on this pull request.
> +
+"math" # don't know how this ends up, but dir="auto" is probably
correct
+ ].each do |name|
+define_method :"convert_#{name}" do |el, indent|
+ attr_bak = el.attr.dup # can't avoid mutating the attr hash, so make
a backup
+ el.attr["dir"] = "auto" unless el.attr.key?("dir") # if by some
miracle dir is already defined, don't override it
+ ret = super(el, indent)
+ el.attr.replace(attr_bak) # restore backup
+ ret
+end
+ end
+
+ # only add dir="auto" to bare links
+ def convert_a(el, indent)
+attr_bak = el.attr.dup
+el.attr["dir"] = "auto" if !el.attr.key?("dir") && el.attr["href"] ==
inner(el, indent)
You're right, the autolinker needs to apply `dir="auto"` or `dir="ltr"` too. My
code will pretty much only affect links made with the syntax
`
Re: [openstreetmap/openstreetmap-website] Set dir="auto" on all user-generated-content (Issue #3428)
NeatNit left a comment (openstreetmap/openstreetmap-website#3428) Thought I'd mention: - Rich plain text (Discussions on changesets, notes; pictured in the screenshots) fixed in #5835. - Markdown (User Diaries, elsewhere?) would be fixed by #5840 - Misc things would be fixed by #3429 (which things?) -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/3428#issuecomment-2791127261 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 dir="auto" to Markdown content for bidi (PR #5840)
NeatNit left a comment (openstreetmap/openstreetmap-website#5840) @gravitystorm or @tomhughes Perhaps you could review? -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840#issuecomment-2827099760 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 dir=auto to user generated content (PR #3429)
NeatNit left a comment (openstreetmap/openstreetmap-website#3429) Thank you @tomhughes for merging #5840 today! Let me know if there's any way I could help with this one. In particular, there are small things I've missed in my PRs because I didn't have the capacity to test properly. Namely: - Title of diary entries, both in [lists of diary entries](https://master.apis.dev.openstreetmap.org/diary/he) and in [the diary entry itself](https://master.apis.dev.openstreetmap.org/user/NeatNit_Dev/diary/39) (the `!` should appear at the left end of the title) - Changeset description (`comment` tag) in history view, both [general](https://www.openstreetmap.org/history) and [user-specific](https://www.openstreetmap.org/user/NeatNit/history?after=163583490) - see screenshots: Wrong:  Right:  I think this PR might already deal with at least one of those. Bug again, no capacity for me to test. Also, semi-relatedly, I noticed that the title of a diary entry has "" in it. For example, with the website in Hebrew the title of [this page](https://www.openstreetmap.org/user/pehar_11/diary/406634) is: ```html היומן שלpehar_11 | [u tijeku] Uređenje centra Rijeke | OpenStreetMap ``` Clearly something needs fixing there. Haven't looked into it yet. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/3429#issuecomment-2828944354 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 dir="auto" to Markdown content for bidi (PR #5840)
NeatNit left a comment (openstreetmap/openstreetmap-website#5840) @AntonKhorev Can I ping you to review this? I don't expect to need more changes. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840#issuecomment-2807334975 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] Set dir="auto" on changeset comment bodies (Issue #5785)
NeatNit created an issue (openstreetmap/openstreetmap-website#5785) ### URL https://www.openstreetmap.org/changeset/163401189 ### How to reproduce the issue? This is a sub-issue of #3428. Some keywords for search: RTL bidi bidirectional LTR Comments inherit the directionality of the entire website instead of the directionality of their content. Example screenshot:  Each comment should have `dir="auto"` in its HTML. Poking around the source (which I have never worked with before, so this may be wrong), I think I've found the relevant code: https://github.com/openstreetmap/openstreetmap-website/blob/a0768a4d04d3eb5afa9e8864b8fc73a64286bf2c/app/views/changesets/show.html.erb#L62-L64 So there is a "band-aid" solution and a more involved "full" solution that would produce better results. Band-aid solution takes two steps: 1. Add `dir="auto"` to the above snippet: ```erb <%= comment.body.to_html %> ``` 2. Add `text-align: start;` to the `mx-2` class CSS. Result:  Perfect. The full solution would take into account the fact that different paragraphs should have different directionality. For example, if I write: ``` This line is English and should be LTR. השורה הזאת בעברית וצריכה להיות מימין לשמאל. השורה הזאת מערבבת LTR ו-RTL אבל מתחילה ב-RTL ולכן צריכה להיות מימין לשמאל. ``` Github would render it correctly: > This line is English and should be LTR. > > השורה הזאת בעברית וצריכה להיות מימין לשמאל. > > השורה הזאת מערבבת LTR ו-RTL אבל מתחילה ב-RTL ולכן צריכה להיות מימין לשמאל. Each of these lines is `` so behaves correctly. However it seems that `comment.body.to_html` *does not* split the text into different `` elements, it's instead a single `` with a single text block containing `` for line splits. **This is not usable for expected formatting.** You would have to replace this function with one that produces separate elements (preferably ``), each with `dir="auto"`. Couple with appropriate `text-align` CSS property, and the result would be ideal. I know that changeset comments are not Markdown and should not be parsed as such, but for acceptable results they have to at least be split into `` elements instead of using `. You can use styling to make it appear visually the same as before. ### Screenshot(s) or anything else? _No response_ -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5785 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] linkify ambiguity (Issue #5883)
NeatNit left a comment (openstreetmap/openstreetmap-website#5883)
Okay, seems like I was looking in the wrong places. Those rich text linkify
calls do call the function in the parent class.
The linkify defined in ApplicationHelper is called from other places. In that
case, is it really a good idea to define effectively the same method but with
subtle differences in separate places, and give it exactly the same name?
Perhaps one can be made an alias of the other?
The helpful folks over at the Ruby Discord server helped me figure this out:
> my guess: you're getting confused as to who is calling what. The `linkify`
> calls in `rich_text.rb` are calling each other whereas the `linkify` calls in
> app are not calling the linkify methods in `rich_text.rb`... I mostly see
> this as very bad naming and that should be addressed.
>
> Here's all the views and the calls to the helpers' `linkify`:
>
> ```
> app/helpers/application_helper.rb
> 4: def linkify(text)
>
> app/helpers/browse_tags_helper.rb
> 43: safe_join(value.split(";", -1).map { |x| linkify(h(x)) }, ";")
>
> app/views/changesets/show.html.erb
> 7:<%= linkify(@changeset.tags["comment"].to_s.presence ||
> t("browse.no_comment")) %>
>
> app/views/changesets/index.atom.builder
> 66: tag_tr.td "#{tag[0]} = #{linkify(tag[1])}"
>
> app/views/browse/_common_details.html.erb
> 12:<%= linkify(common_details.changeset.tags["comment"]) %>
> ```
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5883#issuecomment-2778332416
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 dir="auto" to Markdown content for bidi (PR #5840)
@NeatNit commented on this pull request. > @@ -96,7 +136,7 @@ def to_text class Markdown < Base def to_html - linkify(sanitize(document.to_html), :all) + linkify(sanitize(document.to_html_bidi), :all) https://kramdown.gettalong.org/rdoc/Kramdown/Parser/Base.html > Implementing a new parser is rather easy: just derive a new class from this > class and put it in the > [Kramdown::Parser](https://kramdown.gettalong.org/rdoc/Kramdown/Parser.html) > module – the latter is needed so that the auto-detection of the new parser > works correctly. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840#discussion_r2009231147 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 dir="auto" to Markdown content for bidi (PR #5840)
NeatNit left a comment (openstreetmap/openstreetmap-website#5840) Added `dir="auto"` to the other `linkify` (see #5883), ready for review again. As an aside, I noticed that none of my changes in `rich_text.rb` affected any tests, perhaps tests should be written for it. I could try to do that, but I feel like I'd do it wrong. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840#issuecomment-2778645753 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 dir="auto" to Markdown content for bidi (PR #5840)
@NeatNit pushed 1 commit. 9f9ad0a9ed9156c7b0146fb61789bae1e1ce1b8f add dir="auto" to linkified links and replicate openstreetmap#5850 in ApplicationHelper -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840/files/280a4cf451a90ad4a127bdfdf2e42f82d2f3d256..9f9ad0a9ed9156c7b0146fb61789bae1e1ce1b8f 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 dir="auto" to Markdown content for bidi (PR #5840)
@NeatNit pushed 1 commit. 9feb1f4b404bf2c7d9b659b281e470e6f04bba51 add dir="auto" to linkified links and replicate openstreetmap#5850 in ApplicationHelper -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840/files/9f9ad0a9ed9156c7b0146fb61789bae1e1ce1b8f..9feb1f4b404bf2c7d9b659b281e470e6f04bba51 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 dir="auto" to Markdown content for bidi (PR #5840)
@NeatNit pushed 1 commit. 280a4cf451a90ad4a127bdfdf2e42f82d2f3d256 add dir="auto" to linkified links and replicate #5850 in ApplicationHelper -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840/files/f4972db233c2d08a8494086c5ab935ad38685668..280a4cf451a90ad4a127bdfdf2e42f82d2f3d256 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 dir="auto" to Markdown content for bidi (PR #5840)
@NeatNit pushed 1 commit. 16168bb4d7bd1bdfbc984af1301a0900ae84806f make markdown bidirectional with dir="auto" -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840/files/8284a308c4bfa880720367e6665e4cb66e48cfba..16168bb4d7bd1bdfbc984af1301a0900ae84806f 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] Set dir="auto" on changeset comment bodies (Issue #5785)
NeatNit left a comment (openstreetmap/openstreetmap-website#5785) > It would be ideal if we can find ways to fix this issue, so that it is fixed > by default when developers add new pages or add features to existing pages. > That might be hard to achieve though. Perhaps you'd be able to answer then - how much of user-generated content uses the `to_html` functions defined in the file I quoted? It's very possible that adding `dir="auto"` where I suggested, as well as in the other `to_html` functions in the same file, will actually accomplish most of that goal. The other thing to fix is the CSS, which again might be possible to do in one place that would propagate everywhere, but one would have to be very familiar with the website's structure to know. I'm not actually a web developer, I just know a thing or two, but I don't have the skillset to find exactly where it's best to apply each CSS rule. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5785#issuecomment-2740680261 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 dir="auto" to paragraphs of formatted text (PR #5835)
NeatNit left a comment (openstreetmap/openstreetmap-website#5835) > If you do this, you'd also set `dir=auto` on the entire comment. This would not be useful. Each paragraph needs to have `dir="auto"` separately. Different paragraphs in the same comment can have a different direction. This is why it needs to be an argument to `simple_format`. Thanks for the tip though, I will look into how to do that... Might not have to be in the same PR though. I don't want this PR to grow to the size of #3429 handling each case separately. Perhaps your suggestion should be added there instead. As I said, even without `text-align`, just having `dir="auto"` already offers the most important benefit by correcting the placement of punctuation and mixed directionality text. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5835#issuecomment-2744517853 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 dir="auto" to paragraphs in rich_text.rb (PR #5835)
NeatNit left a comment (openstreetmap/openstreetmap-website#5835) For kramdown, this approach should work: https://codeberg.org/NeatNit/kramdown-bidi/src/branch/main/code.rb (the code works for sure but I haven't tested the results in a browser) I've stayed up too late again... I'll add it to this PR tomorrow :) Thanks to the Ruby Discord for helping me with syntax. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5835#issuecomment-2745906943 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 dir="auto" to Markdown content for bidi (PR #5840)
NeatNit left a comment (openstreetmap/openstreetmap-website#5840) > I thought the previous PR was handling all rich text... I'm sorry I didn't make that clear enough. Changeset comments and notes were the main (only?) thing affected by the previous PR. Legacy or not, they are extremely prominent and important. Are there any plans to move them to Markdown? I did mention in the comments of the previous PR that I'll postpone Markdown for a separate PR, because at the time it seemed out of reach for me. This is that. I got across that gap faster than I thought. Again, sorry it wasn't clear enough. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840#issuecomment-2746410311 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 dir="auto" to Markdown content for bidi (PR #5840)
@NeatNit pushed 1 commit. 8284a308c4bfa880720367e6665e4cb66e48cfba make markdown bidirectional with dir="auto" -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840/files/3fd1fd04a82f9fb8b780b7b7f4f717f018da9023..8284a308c4bfa880720367e6665e4cb66e48cfba 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 dir="auto" to Markdown content for bidi (PR #5840)
@NeatNit commented on this pull request. > @@ -1,5 +1,45 @@ # frozen_string_literal: true +module Kramdown https://kramdown.gettalong.org/rdoc/Kramdown/Parser/Base.html > Implementing a new parser is rather easy: just derive a new class from this > class and put it in the > [Kramdown::Parser](https://kramdown.gettalong.org/rdoc/Kramdown/Parser.html) > module – the latter is needed so that the auto-detection of the new parser > works correctly. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840#discussion_r2009231305 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 dir="auto" to paragraphs in rich_text.rb (PR #5835)
@NeatNit pushed 1 commit. 83575bd9baee1e49d3e3d822d8fd01a423cb1596 Merge branch 'master' into rtl-fixes -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5835/files/b4cae9b7a556cc093138fb728b8bced879d8d773..83575bd9baee1e49d3e3d822d8fd01a423cb1596 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 dir="auto" to paragraphs in rich_text.rb (PR #5835)
NeatNit left a comment (openstreetmap/openstreetmap-website#5835) Looks like I missed something important (main changeset comment). I'll lump that in with my next PR, which handles Markdown. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5835#issuecomment-2746379622 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 dir="auto" to Markdown content for bidi (PR #5840)
@NeatNit commented on this pull request. > @@ -96,7 +136,7 @@ def to_text class Markdown < Base def to_html - linkify(sanitize(document.to_html), :all) + linkify(sanitize(document.to_html_bidi), :all) Oops, wrong link. This is the one: https://kramdown.gettalong.org/rdoc/Kramdown/Converter/Base.html > Implementing a new converter is rather easy: just derive a new class from > this class and put it in the > [Kramdown::Converter](https://kramdown.gettalong.org/rdoc/Kramdown/Converter.html) > module (the latter is only needed if auto-detection should work properly). Basically the same thing. kramdown converts "html_bidi" to "HtmlBidi" and picks the class using that. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840#discussion_r2009234567 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] Incorrect translation choice time_ago for 21 hours ago (Issue #5878)
NeatNit left a comment (openstreetmap/openstreetmap-website#5878)
> > * nothing instead of "%{count}", probably in languages without indefinite
> > article
>
> That's interesting, do you have examples? I wonder if these languages just
> have a choice between saying "day ago" vs "1 day ago" or whether they
> strictly can't use the number. I understand that "day ago" instead of "a day
> ago" is common, but I haven't found if there are languages where you can't
> use a number 1 with a noun.
This is pretty much the case in Hebrew. Forcing in the 1 is readable but feels
kinda awkward.
Natural: לפני יום = a day ago. There is no "a" in Hebrew.
Still pretty natural: לפני יום אחד = one day ago.
Feels unnatural: לפני יום 1 (with digit 1 instead of the word one) - honestly
it *could* be just a personal thing for me but this feels wrong to read.
Less unnatural to read, but completely unnatural to say: לפני 1 יום - again,
this might be a personal thing for me.
The thing is, for 1 the word order is "ago day [one]", but for any other number
of days it's "ago {number} days". This is why the digit 1 has no natural place.
Incidentally, it seems [[the current Hebrew
translation]](https://translatewiki.net/wiki/Osm:Datetime.distance_in_words_ago.x_days/he)
uses "yesterday" and the word for "the day before yesterday", which you
explicitly say isn't ideal.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5878#issuecomment-2776519121
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 TagHelper include from RichText module (PR #5850)
NeatNit left a comment (openstreetmap/openstreetmap-website#5850) I noticed that linkified links only have `rel="nofollow"` instead of the full `rel="nofollow noopener noreferrer"`. I don't think this is my browser or extensions doing funky stuff because I see it in the raw response data in the Network tab of Firefox's dev tools. Is this supposed to happen? -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5850#issuecomment-2776293359 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] Non-markdown rich text uses the wrong linkify (Issue #5883)
NeatNit created an issue (openstreetmap/openstreetmap-website#5883) I noticed that linkified links only have `rel="nofollow"` instead of the full `rel="nofollow noopener noreferrer"`. I don't think this is my browser or extensions doing funky stuff because I see it in the raw response data in the Network tab of Firefox's dev tools. Is this supposed to happen? _Originally posted by @NeatNit in https://github.com/openstreetmap/openstreetmap-website/issues/5850#issuecomment-2776293359_ (I will edit this issue after creating it) -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/5883 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 TagHelper include from RichText module (PR #5850)
NeatNit left a comment (openstreetmap/openstreetmap-website#5850) I see... So when is the Rich Text one used? Seems weird that even rich text seems to get the ApplicationHelper one. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5850#issuecomment-2776471390 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 dir="auto" to Markdown content for bidi (PR #5840)
NeatNit left a comment (openstreetmap/openstreetmap-website#5840) I'm considering rewriting this from scratch, with the following changes which would result in nicer code and better output: 1. Instead of subclassing the HTML converter, I will modify the document upon its creation. This would mean no longer touching the Kramdown namespace. 2. As I explain [here](https://github.com/whatwg/html/issues/10097#issuecomment-2746324449), `dir="auto"` is just not powerful enough to produce optimal results. I would like to implement server-side the algorithm I consider optimal, and set explicit `dir="ltr"` or `dir="rtl"` on each element for which it is relevant. This would undoubtedly create the best bidi markdown implementation the world has ever seen! -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840#issuecomment-2751577925 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 dir="auto" to Markdown content for bidi (PR #5840)
@NeatNit commented on this pull request.
> +"dl",
+# "dd", "dt", # child - since it's similar to a list, it has the same
behavior as a list
+
+"math" # don't know how this ends up, but dir="auto" is probably
correct
+ ].each do |name|
+define_method :"convert_#{name}" do |el, indent|
+ attr_bak = el.attr.dup # can't avoid mutating the attr hash, so make
a backup
+ el.attr["dir"] = "auto" unless el.attr.key?("dir") # if by some
miracle dir is already defined, don't override it
+ ret = super(el, indent)
+ el.attr.replace(attr_bak) # restore backup
+ ret
+end
+ end
+
+ # only add dir="auto" to bare links
+ def convert_a(el, indent)
Yes.
טקסט `func(arg, 1235, "א")` טקסט
the codespand should render as: `func(arg, 1235, "א")`
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5840#discussion_r2015669952
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 dir="auto" to Markdown content for bidi (PR #5840)
@NeatNit pushed 1 commit. 440e1fe705ad630e55af8007b178f786e036c238 make markdown bidirectional with dir="auto" -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840/files/2a24d8abe8bf6b049dfe3e5726884a0f4eec0d5b..440e1fe705ad630e55af8007b178f786e036c238 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 dir="auto" to Markdown content for bidi (PR #5840)
@NeatNit pushed 3 commits. 59e6081aed44d88bc37ca7e6603fd82fc1d80983 add dir="auto" to changeset comment 14a0df4bb55b80e9c8142f2feda7f463bea50bf3 make markdown bidirectional with dir="auto" f4972db233c2d08a8494086c5ab935ad38685668 add dir="auto" to linkify links -- View it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5840/files/fe6262fe0c4ab0f53c329477534c4bf08464ffc1..f4972db233c2d08a8494086c5ab935ad38685668 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 dir="auto" to paragraphs in rich_text.rb (PR #5835)
NeatNit left a comment (openstreetmap/openstreetmap-website#5835) If you have the option to squash and merge through GitHub, that would mean I made a successful PR without ever cloning the repository to my machine. Can you? -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/5835#issuecomment-2746272145 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] bundle version mismatch - can't install with INSTALL.md instructions (Issue #6086)
NeatNit created an issue (openstreetmap/openstreetmap-website#6086) I'm on Linux Mint 22.1, which is based on Ubuntu 24.04, so I expected the install instructions to work. https://github.com/openstreetmap/openstreetmap-website/blob/48f864f98a2cd072d7730036a43d5c6aeae13bc4/INSTALL.md?plain=1#L12 The biggest difference between Mint and Ubuntu is that Mint disables the snap store - are snaps required for this installation to work? This part worked fine: https://github.com/openstreetmap/openstreetmap-website/blob/48f864f98a2cd072d7730036a43d5c6aeae13bc4/INSTALL.md?plain=1#L30-L40 I then cloned the repository and tried `bundle install`: https://github.com/openstreetmap/openstreetmap-website/blob/48f864f98a2cd072d7730036a43d5c6aeae13bc4/INSTALL.md?plain=1#L128-L133 ``` $ bundle install Bundler 2.4.20 is running, but your lockfile was generated with 2.6.2. Installing Bundler 2.6.2 and restarting using that version. Fetching gem metadata from https://rubygems.org/. There was an error installing the locked bundler version (2.6.2), rerun with the `--verbose` flag for more details. Going on using bundler 2.4.20. Fetching gem metadata from https://rubygems.org/ Fetching https://github.com/brianhempel/active_record_union.git There was an error while trying to write to `/var/lib/gems/3.2.0/cache/bundler/git`. It is likely that you need to grant write permissions for that path. ``` I then tried a few things to try to fix this to no avail, but I didn't try running bundle with sudo as that sounds like it shouldn't be necessary. Before I make a bigger mess of things - are you sure these instructions work as-is on Ubuntu 24.04? Perhaps more steps are needed? -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/6086 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] bundle version mismatch - can't install with INSTALL.md instructions (Issue #6086)
NeatNit left a comment (openstreetmap/openstreetmap-website#6086) Maybe the COPY lines are needed? https://github.com/NeatNit/openstreetmap-website/blob/48f864f98a2cd072d7730036a43d5c6aeae13bc4/Dockerfile#L44-L51 I've never used Docker so I'm pretty clueless, honestly. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/6086#issuecomment-2940618764 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] bundle version mismatch - can't install with INSTALL.md instructions (Issue #6086)
NeatNit left a comment (openstreetmap/openstreetmap-website#6086) @tomhughes I was under the impression that the INSTALL.md instructions should at least work: https://github.com/openstreetmap/openstreetmap-website/issues/5733#issuecomment-2691272414 > we try to support developers with fresh installs, up to the point that they > can get the tests running. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/6086#issuecomment-2940649085 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] bundle version mismatch - can't install with INSTALL.md instructions (Issue #6086)
NeatNit left a comment (openstreetmap/openstreetmap-website#6086) > Please understand that installing and running the code in this repository is > requires a degree of technical ability - it's not really something that > non-technical end users are likely to find very easy. Of course, I understand this. Technical ability is not the issue, rather, the issue is unfamiliar technologies. In a couple of weeks of toying with this I might understand all of this well enough to breeze through it, but before that I need to get up and running. That's the point of installation instructions, isn't it? > Try something like this in he top level of your checkout: > > ``` > bundle config set path vendor/bundle > ``` > > which should configure bundler to install to the `vendor/bundle` directory. Thanks, this seems to have worked. I've now hit the mini_racer problem, I'll look into that issue for how to resolve it. Consider adding this to INSTALL.md. -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/issues/6086#issuecomment-2940845013 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
