Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

💥 feat: force code blocks LTR rendering #412

Merged
merged 1 commit into from
Oct 18, 2024
Merged

Conversation

welpo
Copy link
Owner

@welpo welpo commented Oct 15, 2024

💥 Breaking change

Code blocks now default to LTR rendering, regardless of the page's text direction. RTL language users who relied on RTL code blocks will need to explicitly set force_codeblock_ltr = false to maintain their current behaviour.

Summary

Adds a new configuration option force_codeblock_ltr to control the text direction of code blocks. When enabled (default), it forces left-to-right (LTR) rendering for all code content, including code blocks and inline code.

The setting follows the hierarchy.

Related issue

Fixes #411.

Changes

  • Added new configuration option force_codeblock_ltr (default: true)
  • Implemented CSS rules to force LTR rendering for code when the option is enabled
  • Updated documentation to include the new option and its usage

Type of change

  • Bug fix (fixes an issue without altering functionality)
  • New feature (adds non-breaking functionality)
  • Breaking change (alters existing functionality)
  • UI/UX improvement (enhances user interface without altering functionality)
  • Refactor (improves code quality without altering functionality)
  • Documentation update
  • Other (please describe below)

Checklist

  • I have verified the accessibility of my changes
  • I have tested all possible scenarios for this change
  • I have updated theme.toml with a sane default for the feature
  • I have made corresponding changes to the documentation:
    • Updated config.toml comments
    • Updated theme.toml comments
    • Updated "Mastering tabi" post in English
    • Updated "Mastering tabi" post in Spanish
    • Updated "Mastering tabi" post in Catalan

@welpo welpo added the i18n Internationalisation label Oct 15, 2024
@welpo
Copy link
Owner Author

welpo commented Oct 15, 2024

@TheAwiteb I'd appreciate it if you could take a look and share your thoughts here

Copy link

netlify bot commented Oct 15, 2024

Deploy Preview for tabi-demo ready!

Name Link
🔨 Latest commit 35614b0
🔍 Latest deploy log https://app.netlify.com/sites/tabi-demo/deploys/6711a7196f5fc70008c4942d
😎 Deploy Preview https://deploy-preview-412--tabi-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@TheAwiteb
Copy link
Collaborator

I agree with code and not pre, because code often contains programming code, which is often LTR and rarely RTL, but pre doesn't have to be LTR

@TheAwiteb
Copy link
Collaborator

TheAwiteb commented Oct 15, 2024

Also, I don't think there is a need for it. There is no need to hard code the direction. You can add a div with the direction you want before the code block.

<div dir="ltr">

```rust
fn main() {
    println!("Hello, World");
}
```\

</div>

@welpo welpo force-pushed the fix/force-ltr-code-blocks branch from fdee8fb to ed8097f Compare October 15, 2024 21:48
@welpo
Copy link
Owner Author

welpo commented Oct 15, 2024

Thanks for your thoughts, @TheAwiteb!

I think it makes sense to support the most common use cases, both for RTL and LTR users.

Would it be fair to assume more than half of the time, the author of a page will want LTR code block? The div idea is good, and could be used in case someone needs an RTL code block. In this case, I would keep the "force LTR" in tabi.

To me, forcing code blocks to LTR but not inline code makes sense. Thoughts, @mohammad-amin-khajeh?

@TheAwiteb
Copy link
Collaborator

The div idea is good, and could be used in case someone needs an RTL code block.

If you force it in CSS I don't think you can make it RTL right? So the div idea won't work

@welpo
Copy link
Owner Author

welpo commented Oct 15, 2024

If you force it in CSS I don't think you can make it RTL right? So the div idea won't work

You're right.

I'm thinking of:

  • Changing nothing, and users can use the div trick
  • Adding a config.toml option to force code blocks to RTL

Would love your thoughts, @TheAwiteb @mohammad-amin-khajeh

@TheAwiteb
Copy link
Collaborator

Changing nothing, and users can use the div trick

For me, this is better

Adding a config.toml option to force code blocks to RTL

Not all code blocks in a post need to be RTL, it can be both, LTR and RTL

@mohammad-amin-khajeh
Copy link

mohammad-amin-khajeh commented Oct 16, 2024

Changing nothing, and users can use the div trick

I feel like this will get repetitive and tedious quickly, because most people I assume(writing in an rtl language that is) would want to have their codeblocks ltr.

I suggest making codeblocks ltr by default, while also having a shortcode that behaves like add_src_to_code_block which makes the codeblock below it rtl in case someone wants that.

@welpo
Copy link
Owner Author

welpo commented Oct 16, 2024

How about:

  • hierarchy-respecting config to force LTR codeblocks (on by default)
  • shortcode to force LTR/RTL content within

@TheAwiteb
Copy link
Collaborator

shortcode to force LTR/RTL content within

Shortcode looks better, something like this

{% ltr() %}

...

{% end %}

@welpo
Copy link
Owner Author

welpo commented Oct 16, 2024

I will add the shortcode.

Would a default LTR codeblock setting make sense / satisfy most users? Again, with the option to unset / override for the whole site/section/specific page.

Essentially, the question is: would most RTL website owners expect their codeblocks to be LTR by default?

@mohammad-amin-khajeh
Copy link

mohammad-amin-khajeh commented Oct 16, 2024

I can't speak of everyone here.

But I don't think I personally will ever change it to RTL if it were LTR by default.
Because slashes in urls and hashes and a lot of other common symbols would look messy and all over the place.
Just as you saw in the picture I included in the issue.

So to me it makes total sense if LTR was the default.

@TheAwiteb
Copy link
Collaborator

would most RTL website owners expect their codeblocks to be LTR by default?

Yes

@welpo welpo force-pushed the fix/force-ltr-code-blocks branch from ed8097f to da5b228 Compare October 17, 2024 23:53
@welpo welpo changed the title 💄 style: force LTR rendering for code blocks ✨ feat: force LTR rendering for code blocks Oct 17, 2024
@welpo welpo changed the title ✨ feat: force LTR rendering for code blocks 💥 feat: force LTR rendering for code blocks Oct 17, 2024
- Add new config option `force_codeblock_ltr` (default: true)
- Implement CSS to force LTR rendering of code blocks when enabled
- Update documentation to include new option

BREAKING CHANGE: Code blocks default to LTR. Set `force_codeblock_ltr = false` to keep previous behaviour.
@welpo welpo force-pushed the fix/force-ltr-code-blocks branch from da5b228 to 35614b0 Compare October 18, 2024 00:08
@welpo
Copy link
Owner Author

welpo commented Oct 18, 2024

I will review the PR soon. The shortcode(s) to force LTR/RTL will come in another PR.

@welpo welpo merged commit 9859b12 into main Oct 18, 2024
8 checks passed
@welpo welpo deleted the fix/force-ltr-code-blocks branch October 18, 2024 14:34
@welpo welpo changed the title 💥 feat: force LTR rendering for code blocks 💥 feat: force code blocks LTR rendering Oct 18, 2024
welpo added a commit that referenced this pull request Oct 18, 2024
- Introduce new `force_text_direction` shortcode
- Overrides global `force_codeblock_ltr` setting and document direction
- Accepts "ltr" or "rtl" as direction parameter

Related: #412
welpo added a commit that referenced this pull request Oct 18, 2024
BREAKING CHANGE: Code blocks default to LTR. Set `force_codeblock_ltr = false` to keep previous behaviour.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i18n Internationalisation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add the ability to make codeblocks ltr in rtl languages
3 participants