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

Added RTL Style for Classic Theme #17248

Merged
merged 1 commit into from Jan 20, 2020

Conversation

Progi1984
Copy link
Contributor

@Progi1984 Progi1984 commented Jan 20, 2020

Questions Answers
Branch? 1.7.6.x
Description? Added RTL Style for Classic Theme
Type? bug fix
Category? FO
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #17245
How to test? Cf issue

This change is Reviewable

@Progi1984 Progi1984 requested a review from a team as a code owner January 20, 2020 13:47
@prestonBot prestonBot added 1.7.6.x Branch Bug Type: Bug labels Jan 20, 2020
@@ -0,0 +1,50 @@
#body {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this file is generated and the weird indentation is normal ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@PierreRambaud PierreRambaud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RTL shouldn't be commited 🤔
ping @eternoendless

@matks
Copy link
Contributor

matks commented Jan 20, 2020

RTL shouldn't be commited 🤔
ping @eternoendless

RTL BO assets should not be committed 😄
RTL FO Classic Theme assets ought to be, they were commited for 1.7.6.2, see https://github.com/PrestaShop/PrestaShop/tree/1.7.6.2/themes/classic/assets/css

@Progi1984
Copy link
Contributor Author

RTL shouldn't be commited thinking
ping @eternoendless

Actually, no script generate *_rtl file. You must need to generate them from the backoffice. For the next version, an issue #17247 has been created by @matks for automating that.

matks
matks previously approved these changes Jan 20, 2020
PierreRambaud
PierreRambaud previously approved these changes Jan 20, 2020
@Progi1984 Progi1984 added the Waiting for QA Status: action required, waiting for test feedback label Jan 20, 2020
@Progi1984 Progi1984 added this to the 1.7.6.3 milestone Jan 20, 2020
@Progi1984 Progi1984 dismissed stale reviews from PierreRambaud and matks January 20, 2020 14:00

The base branch was changed.

@Progi1984 Progi1984 changed the base branch from develop to 1.7.6.x January 20, 2020 14:00
@Robin-Fischer-PS
Copy link
Contributor

Hi @Progi1984 ,

I tested BO and FO RTL display in Arabic and Persian, with theses languages as default, and with English as default. I also tested a fresh install in Persian (because Arabic is not possible due to another bug).

All my tests are OK, so it's QA ✔️ !

@Robin-Fischer-PS Robin-Fischer-PS added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Jan 20, 2020
@Robin-Fischer-PS Robin-Fischer-PS self-assigned this Jan 20, 2020
Copy link
Member

@eternoendless eternoendless left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These assets must be generated automatically when installing the language and MUST NOT be committed unless you have a very good reason.

Copy link
Member

@eternoendless eternoendless left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok go for merging this but we need to make a decision for 1.7.7

@eternoendless
Copy link
Member

Thank you @Progi1984

@eternoendless eternoendless merged commit 31a252c into PrestaShop:1.7.6.x Jan 20, 2020
@Progi1984 Progi1984 deleted the issue17245 branch January 20, 2020 14:37
@matks
Copy link
Contributor

matks commented Mar 11, 2020

Ok go for merging this but we need to make a decision for 1.7.7

And develop ! 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.7.6.x Branch Bug Type: Bug QA ✔️ Status: check done, code approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants