-
Notifications
You must be signed in to change notification settings - Fork 11
Add support for replacing CSS files with .rtl.css versions for RTL cultures #15
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
Conversation
This is an excellent addition! The ability to automatically load .rtl.css stylesheets for RTL cultures should have been implemented from the beginning. Having framework-level support for culture-specific styling will give us much better control over layouts and templates, especially in multilingual applications. This feature will significantly improve the development experience for RTL languages and will likely encourage more widespread adoption of the platform. It's great to see this finally being addressed after all these years. Thank you for listening to community needs and making this enhancement! |
Thank you for this PR! Honestly, this feature is absolutely essential for any serious multi-lingual DNN implementation that wants to support RTL languages properly. We truly need this in the core! Thank you again for your effort. |
Thanks for suggesting this feature! @mnouraei Keeping .rtl.css files separate—rather than mixing RTL styles into the main CSS—is crucial for building scalable, maintainable, and fast-loading websites that support RTL users. This PR is spot on:
I fully support merging this into the DNN core. |
Automatic support for .rtl.css files for right-to-left (RTL) languages has long been an essential need in multilingual DNN-based projects. As developers working in RTL-first environments for years, we can confidently say that separating RTL styles into dedicated .rtl.css files is not just a best practice — it’s already the de facto standard in real-world Persian and Arabic web projects. This approach significantly: Keeps the main CSS clean and maintainable Loads only what’s necessary for RTL users Improves overall site performance Integrating this feature into the DNN core will greatly enhance the development experience for RTL languages and enable scalable, professional support for RTL users. Thank you for recognizing the real needs of the community and proposing this pull request. |
Hey @valadas |
Appreciation for Your Outstanding Contribution to the PR Project Dear Engineer Noraei, Your dedicated efforts and expertise had a significant impact on the success of the PR project. Your precision, professionalism, and commitment throughout the process were truly commendable. I sincerely thank you for the time and energy you devoted to this work. We truly value the collaboration between you and the DNN team, and we strongly encourage the continuation of this productive partnership in future projects. With appreciation, |
I am not a big fan of serving whole different css just before of text direction myself. That would mean having to duplicate everything else that is not specific to text direction. Another PR was submitted to provide the text direction as an html attribute and a class on the body. I believe css selectors should be used instead of completely different files. |
@valadas In the previous PR that you approved, I mentioned that I am planning to gradually apply my 6 years of experience working with this repository along with over 15 years of expertise in DNN to enhance this system and add proper support for right-to-left languages. This solution has been carefully evaluated over the years and has been successfully used and welcomed by many governmental and non-governmental organizations. Therefore, as someone who has been actively involved in this community for a long time, I believe this feature is highly essential, which is why I proposed adding it to the core after all these years. I kindly ask you to consider this with an open mind and approve this change. |
Thanks for proposing this change! As someone working with RTL languages, I can confirm that separating RTL styles into dedicated .rtl.css files makes a big difference in terms of maintainability and performance. It keeps the main CSS clean and avoids unnecessary bloat, and only loads what's needed for RTL languages, which improves both performance and load times. I fully support merging this PR into the DNN core. Proper RTL support is a practical improvement for many projects, and this solution seems like the right way forward. |
"Great implementation! This approach strikes a perfect balance between performance optimization and maintainability. By isolating RTL-specific styles into separate CSS files, you not only avoid unnecessary bloat in the default CSS but also enhance the scalability of the site for different cultural settings. It's clear that you've taken a practical and future-proof approach, especially with the emphasis on modularity. I also appreciate the note on the long-term successful implementation—always reassuring to see solutions that have stood the test of time. Awesome work!" |
@mnouraei thanks a lot for this valuable contribution |
Is dnnsoftware/Dnn.Platform#6622 still required if this change is in place? It seems like they're both doing the same thing |
dependency.FilePath = basePath + dependency.FilePath; | ||
|
||
// Replace CSS file with its RTL version if the current culture is right-to-left and the RTL file exists | ||
if (System.Globalization.CultureInfo.CurrentCulture.TextInfo.IsRightToLeft && dependency.FilePath.EndsWith(".css", StringComparison.OrdinalIgnoreCase) && !dependency.FilePath.Contains("http")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible/suggested that we need to support this for JS dependencies as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion, it would be great if this feature is supported for JS as well.
However, I haven’t tested this yet to be able to confidently create a PR for it.
I also mentioned in PR dnnsoftware/Dnn.Platform#6622 that these two behaviors are different and this check needs to be done in both places. |
I've pushed a change which makes the RTL replacement happen in all cases (I tested with |
I also made it more general, so that it works with any file extension, not just CSS. |
Very nice, these changes look great. Based on the changes you’ve made, if I understand correctly, we no longer need to modify the ClientResourceManager.cs class (DNN.Platform). Is that correct? |
@bdukes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, after discussions, I agree with this PR.
@mitchelsellers can you give another set of eyes on this (since I rewrote things, my approval probably shouldn't count)? @valadas once this is in, we'll also need to bump the version and release, and then pull that new version in the Dnn.Platform code. |
This change adds functionality to check if the current culture is right-to-left (RTL) and replace the CSS file with its corresponding .rtl.css version if it exists.
This approach allows RTL-specific styles to be kept in separate CSS files rather than merging them into the main CSS, which:
In summary, it optimises loading for RTL cultures by using dedicated .rtl.css files when available.
Note: This approach has been tested and used for over 4 years in the repository below without issues:
Persian-DnnSoftware/ClientDependency