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

Site Logo: Updates site-logo-control-rtl.css using autortl #13651

Closed
wants to merge 1 commit into from

Conversation

creativecoder
Copy link
Contributor

@creativecoder creativecoder commented Oct 4, 2019

Changes proposed in this Pull Request:

The site logo module is shared between Jetpack and wpcom, but is not synced automatically.

I recently updated wpcom code to bring it into sync with Jetpack (D33102-code). One file that I could not update was site-logo-control-rtl.css, because it is automatically generated by autortl. This PR updates that file in Jetpack with the autortl generated version.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

  • See p9dueE-114-p2 for context about the site logo module

Testing instructions:

  • Install and activate a theme that supports Jetpack's site-logo, such as Canape
  • Go to Settings > General in wp-admin and change the site language to an rtl language, such as Arabic
  • Load the customizer and select the "Site Identity" section (first one)
  • See that the site logo Customizer control is correctly aligned on the right side of the screen.

Proposed changelog entry for your changes:

  • Updated rtl css for site logo module using autortl

@creativecoder creativecoder requested review from a team, scruffian and pablinos October 4, 2019 01:25
@jetpackbot
Copy link

Warnings
⚠️

The PR is missing at least one [Status] label. Suggestions: [Status] In Progress, [Status] Needs Review

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against d537f5a

@creativecoder creativecoder added the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Oct 4, 2019
@jeherve jeherve added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Focus] i18n Internationalization / i18n, adaptation to different languages [Feature] Theme Tools labels Oct 4, 2019
@jeherve
Copy link
Member

jeherve commented Oct 4, 2019

This is not necessary in Jetpack, as the RTL and minified stylesheets are automatically generated when the built version of Jetpack is created before a release. As a result, your changes would be reverted before the next release.

I did, however, see that the RTL and minified versions were not enqueued when necessary. I created #13654 to fix that.

@jeherve jeherve closed this Oct 4, 2019
@jeherve jeherve removed the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Oct 4, 2019
@creativecoder
Copy link
Contributor Author

I did, however, see that the RTL and minified versions were not enqueued when necessary. I created #13654 to fix that.

Thanks!

@scruffian scruffian deleted the update/site-logo-control-rtl-css branch October 7, 2019 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Theme Tools [Focus] i18n Internationalization / i18n, adaptation to different languages [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants