Plume-org / Plume

Federated blogging application, thanks to ActivityPub (now on https://git.joinplu.me/ — this is just a mirror)
https://joinplu.me
GNU Affero General Public License v3.0
2.09k stars 133 forks source link

Add RTL support (BIDI) #575

Open ahangarha opened 5 years ago

ahangarha commented 5 years ago

Plume simply is LTR suited. It doesn't follow direction based on the language. Therefore, If a post be made in RTL languages (such as Persian, Arabic, Urdu or Hebrew) the text is still shown in LTR. I just tested it on http://fediverse.blog/ and the result is as following:

RTL problem on plume v0.3.0

ahangarha commented 5 years ago

I tried to work on this issue. I need some help with understanding template in rust. I couldn't figure out where to find related stuff to@Html(&article.content) which is in templates/posts/details.rs.html.

Moreover, I couldn't find the workflow for building and testing the project after committing changes.

I do appreciate any help. Otherwise, it seems I cannot continue!

trinity-1686a commented 5 years ago

Html(..) means whatever is inside won't get escaped so there can be html content in it (otherwise pictures, links and so on would be escaped instead of shown properly). article is a Post structure (defined in plume-models/src/posts.rs around line 40), and content is it's field containing the html-rendered body of the article. I think your best bet would be to add dir="auto" to the surrounding <article>, if there is need for more changes it will probably be in the Rust part.

If you have installed a test instance, to rebuild, just stop the running one and run cargo run, it will recompile what needs to. The other (maybe easier way) is to make a pull request. After a few minutes it should appear here, and it will be updated with each new commit you push

ahangarha commented 5 years ago

Actually adding dir="auto" would make whole of the article follow direction based on the first letter on the article. As per my experience, this is not going to work properly. I have already put it for H1 and H2, but it should be added to each and every paragraph tags in the post content.

This is the result of applying dir=auto to article tag:

applying `dir=auto` to article tag

If I be right, this should be handled from markdown interpreter. Right?

For test, I can send WIP PR. would it work?

trinity-1686a commented 5 years ago

I think you are right. Yes you can make WIP PR, just write it's WIP in the title or the first message so we know it's not ready for review yet :)

ahangarha commented 5 years ago

So I will apply dir="auto" to article for the time being till we can solve this issue permanently.

ahangarha commented 4 years ago

May you guide me for this issue: where is the MD to HTML engine? I need to work on that to add bidi support. Logically it is out of this project, but where?

trinity-1686a commented 4 years ago

the markdown processor we use is pulldown-cmark

ahangarha commented 4 years ago

The issue is opened on pulldown-cmark repo and I try chance to apply changes. if they accept, it would be a great step ahead

ahangarha commented 3 years ago

I have recently published an article which is in Persian (RTL) but contains some codes (LTR). There you can see how lack of bidi support by the markdown parser make damage to the article.

I have tried in past to convince them to add bidi support but I am not sure when would that happen. Would you consider adding a little js file to add bidi support only for article body? (the rest of plume support into a good extent)

trinity-1686a commented 3 years ago

I've forked pulldown-cmark and proposed a pull-request to Plume that should fix this issue. This is less than ideal, but I'm getting bored that this issue is still here while being so basic. I understand their requirement regarding the specification, but Plume by not being a library has the flexibility to not follow the spec when it doesn't want, and I think this is one of those times we shouldn't.

ahangarha commented 3 years ago

This is a great idea. maybe Plume can become a model for them to see this proposal is practical.

KitaitiMakoto commented 3 years ago

Plum 0.6.0 should include fixes for this issue but I missed, sorry. Latest source code includes the fixes and fediverse.blog uses it now. So, you can try on the instance. For existing blog posts, you need go edit panel and just save them without doing anything.

ahangarha commented 3 years ago

Latest source code includes the fixes and fediverse.blog uses it now. So, you can try on the instance. For existing blog posts, you need go edit panel and just save them without doing anything.

Just checked. It is working. I will try several scenarios to make sure there would be no bug in Plume 0.6.0

Thanks

KitaitiMakoto commented 3 years ago

Thanks for you confirmation. I am looking forward to hearing the result.

ahangarha commented 3 years ago

So far, everything looks good except on thing: <li> element shouldn't have dir="auto".