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:
-
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 yo
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 ar
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 thing
@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
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 get
@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
### 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 t
@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 th
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/pul
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.
Me
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.co
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
@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
> Implement
@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
@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..280a4cf451a90ad4a
@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..9feb
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, bu
@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..9f9a
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 i
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 respo
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 w
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
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
@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://
@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
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
>
> @d
@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 bac
@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 :"conve
@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 hypothet
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 becaus
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
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 me
@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
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#iss
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
### 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
@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 receivi
@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::Par
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 prominen
@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
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-web
@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/Kr
@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
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
@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 beca
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 compu
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
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 t
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 w
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-
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/openstr
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
@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..b
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/o
@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
@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
@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 subs
@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 jus
@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..6c05f1e9c9757e4d65ec2bef116
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;
tex
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/
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 ca
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 we
64 matches
Mail list logo
