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

#399 Generate RTL Styles for CMB2 #510

Merged
merged 2 commits into from
Feb 28, 2016
Merged

#399 Generate RTL Styles for CMB2 #510

merged 2 commits into from
Feb 28, 2016

Conversation

devinsays
Copy link
Contributor

Pull request for this ticket: #399

You may want to move the RTL style generation and compression into its own Grunt task (or add to build:i18n). I'm not sure how much extra time it adds if you're running watch, and is probably only necessary on build.

@jtsternberg
Copy link
Member

Hey @devinsays, did we get any traction testing this? If not, we may want to just roll it in, and get feedback organically.

@devinsays
Copy link
Contributor Author

I had one person offer to do testing, but they needed me to set up a public dev environment for them so they could look. Just haven't had a chance to do that yet.

A number of people requested this feature. It would be great if @hyyan, @MoonMan22, @safiro or @adilelsaeed could jump in and test.

#399

However, I think it would be fine to roll and test organically. Any bugs would likely be minor display issues and probably still an upgrade from the current display in RTL languages.

@wesamly
Copy link

wesamly commented Dec 7, 2015

@devinsays How can download the version contains your updates? I started working with CMB2, and I need RTL for Arabic.

@jtsternberg
Copy link
Member

@wesamly You can download/test the trunk branch.

@wesamly
Copy link

wesamly commented Dec 8, 2015

@jtsternberg Thank you, i'll test and be back with results.

@wesamly
Copy link

wesamly commented Dec 8, 2015

RTL is working great. I tested with Arabic. It will be great if added to master branch. Thank you @jtsternberg @devinsays

screen shot 2015-12-08 at 11 59 31 am

@wesamly
Copy link

wesamly commented Dec 8, 2015

Hello,
I noticed that when I used this branch, I get the following error in some pages:
Fatal error: Cannot redeclare cmb2_dir() (previously declared in /Applications/MAMP/htdocs/lab/wptest/wp-content/themes/cpt_theme/cpt_base/library/CMB2/includes/helper-functions.php:18) in /Applications/MAMP/htdocs/lab/wptest/wp-content/plugins/ls_slider/CMB2/includes/helper-functions.php on line 20

But when I use the official branch, it doesn't appear.

I've a plugin that embed CMB2, and I also embed it in a theme.

@devinsays
Copy link
Contributor Author

I think this is fine to merge assuming you're fine with the implementation. We've had at least a few RTL readers sign off on how it works.

I can't reproduce the the declaration issues, and don't see anything in the changeset that would cause a problem like that.

@jtsternberg Let me know if there's anything else you would like me to test, or if a unit test could/should be added here.

@jtsternberg
Copy link
Member

It is merged (it's in the trunk branch), I just need to take the time to put out a proper release.

@jtsternberg jtsternberg merged commit ce80137 into CMB2:master Feb 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants