Skip to content

Build / JITM: move assets inside package and build all CSS when building plugin #13143

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.

Sign up for GitHub

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

Jump to bottom
Merged
jeherve merged 5 commits into from
Jul 30, 2019
Merged

Build / JITM: move assets inside package and build all CSS when building plugin #13143

jeherve merged 5 commits into from
Jul 30, 2019

Conversation

@jeherve
Copy link
Member

@jeherve jeherve commented Jul 29, 2019 •
edited
Loading

Fixes #13089

Changes proposed in this Pull Request:

  • We will now ship a JITM package (but also all other DNA packages) with an assets folder that will contain all necessary CSS files (minified as well as RTL files) for that package.

Note: I did not include .map files. Should we aim to include those?

Testing instructions:

  • Start from a site running WordPress in Arab, or Hebrew
  • Install and connect Jetpack with this branch.
  • Once connected to WordPress.com, look for pages that will display JITMs. Dashboard > Updates should show one, as well as the main Dashboard, or Appearance > Themes in some cases.
  • The JITMs should be displayed properly.

Proposed changelog entry for your changes:

  • Dashboard Notices: fix layout issues on sites using an RTL language.

@jeherve jeherve added [Type] Bug When a feature is broken and / or not performing as intended [Status] Needs Review This PR is ready for review. [Focus] i18n Internationalization / i18n, adaptation to different languages [Pri] High [Feature] JITM Just In Time Messages - pop-up tips and suggestions that appear on the dashboard and sidebar. Build [Focus] Jetpack DNA labels Jul 29, 2019
@jeherve jeherve added this to the 7.6 milestone Jul 29, 2019
@jeherve jeherve requested review from a team and zinigor July 29, 2019 17:25
@jeherve jeherve self-assigned this Jul 29, 2019
@jetpackbot
Copy link
Collaborator

jetpackbot commented Jul 29, 2019 •
edited
Loading

Warnings
⚠️

pre-commit hook was skipped for one or more commits

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 0a2e28f

zinigor
zinigor approved these changes Jul 29, 2019
View reviewed changes
Copy link
Contributor

@zinigor zinigor left a comment

Choose a reason for hiding this comment

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

Works great, code looks good, thank you!

@zinigor zinigor added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Jul 29, 2019
@jeherve jeherve merged commit 263a29b into master Jul 30, 2019
@jeherve jeherve deleted the add/scss-compiling-packages branch July 30, 2019 10:34
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Jul 30, 2019
jeherve added a commit that referenced this pull request Jul 30, 2019
@jeherve
Changelog: add #13143
9e107a9
jeherve added a commit that referenced this pull request Jul 30, 2019
@jeherve
7.6: running changelog (#12940)
f1d6377 
* Add initial changelog / testing list changes for 7.6

* Update stable tag to 7.5.3

* changelog: add #12957

* Changelog: add #12932

* Changelog: add #12867

* Changelog: add #12823

* changelog: add #12969

* changelog: add #13012

* changelog: add #12974

* Changelog: add #13059

* Changelog: add #13079

* Changelog: add #12924

* changelog: add #12954

* Changelog: add #12959

* Changelog: add #12977

* Changelog: add #12830

* Changelog: add #12926

* Changelog: add #12958

* Changelog: add #12999

* Changelog: add #13077

* Changelog: add #13083

* Changelog: add #13087

* Changelog: add #13110

* Changelog: add #13116

* Changelog: add #13117

* Changelog: add #12821

* Changelog: add #13120

* changelog: add #13139

* Changelog: add #13143

* Changelog: add #13147

* Testing list: add section about sync
jeherve added a commit that referenced this pull request Jul 30, 2019
@jeherve
7.6: running changelog (#12940)
37aabd8 
* Add initial changelog / testing list changes for 7.6

* Update stable tag to 7.5.3

* changelog: add #12957

* Changelog: add #12932

* Changelog: add #12867

* Changelog: add #12823

* changelog: add #12969

* changelog: add #13012

* changelog: add #12974

* Changelog: add #13059

* Changelog: add #13079

* Changelog: add #12924

* changelog: add #12954

* Changelog: add #12959

* Changelog: add #12977

* Changelog: add #12830

* Changelog: add #12926

* Changelog: add #12958

* Changelog: add #12999

* Changelog: add #13077

* Changelog: add #13083

* Changelog: add #13087

* Changelog: add #13110

* Changelog: add #13116

* Changelog: add #13117

* Changelog: add #12821

* Changelog: add #13120

* changelog: add #13139

* Changelog: add #13143

* Changelog: add #13147

* Testing list: add section about sync
@jsnmoon jsnmoon mentioned this pull request Aug 7, 2019
Closed
@jeherve jeherve mentioned this pull request Oct 23, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@zinigor zinigor zinigor approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jeherve jeherve

Labels

Build [Feature] JITM Just In Time Messages - pop-up tips and suggestions that appear on the dashboard and sidebar. [Focus] i18n Internationalization / i18n, adaptation to different languages [Focus] Jetpack DNA [Pri] High [Type] Bug When a feature is broken and / or not performing as intended

Projects

None yet

Milestone

7.6

Development

Successfully merging this pull request may close these issues.

JITM: RTL CSS file is created by build inside css folder, but not inside vendor/automattic/jetpack-jitm/assets

5 participants

@jeherve @jetpackbot @zinigor @matticbot