Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#7007 closed defect (bug) (fixed)

RTL CSS generation: Switch from CSSJanus to RTLCSS

Reported by: netweb Owned by:
Milestone: 2.6 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch needs-testing commit
Cc:

Description

Via upstream change in #WP31332, wp:changeset:31573, #bbPress2848, and bbpress:changeset:5911

Summary

Notes for core development:

  • If you have used grunt cssjanus before, use grunt rtlcss now.

This change replaces all instances of grunt cssjanus with grunt rtlcss

Attachments (5)

7007.patch (3.6 KB) - added by netweb 5 years ago.
7007.1.patch (2.5 KB) - added by netweb 5 years ago.
7007-src.diff (4.5 KB) - added by netweb 5 years ago.
7007-build.diff (11.7 KB) - added by netweb 5 years ago.
7007.results-dc.patch (2.4 KB) - added by dcavins 4 years ago.

Download all attachments as: .zip

Change History (20)

@netweb
5 years ago

#1 @netweb
5 years ago

Patch may need extDot: 'last', added back in, need to test and verify CSS files in:

  • src/bp-messages/css/autocomplete/

#2 @DJPaul
5 years ago

  • Keywords needs-testing added

If someone wants to test and verify this extDot thing, we'll be able to put it into 2.6.

@netweb
5 years ago

@netweb
5 years ago

@netweb
5 years ago

#3 @netweb
5 years ago

Refreshed patch in 7007.1.patch:

7007.1.patch refresh this now includes extDot: 'last':

  • Includes the ignore RTL synax change from CSSJanus' /* @noflip */ to RTLCss' /* rtl:ignore */ in src/bp-activity/css/mentions.css`

7007-src.diff This is the /src folder diff after running grunt release (Includes the original 7007.1.patch)

  • 1st src/bp-activity/css/mentions-rtl.css
  • 2nd src/bp-core/css/buddybar-rtl.css
  • 3rd src/bp-messages/css/autocomplete/jquery.autocompletefb-rtl.css

7007-build.diff This is the /build folder diff after running grunt release

  • 1st bp-core/css/buddybar-rtl.min.css via 2nd /src change above in src/bp-core/css/buddybar-rtl.css
  • 2nd bp-messages/css/autocomplete/jquery.autocompletefb-rtl.min.css via 3rd /src change above in src/bp-messages/css/autocomplete/jquery.autocompletefb-rtl.css
  • There is no new/change/minified from the st /src change above in src/bp-activity/css/mentions-rtl.css or src/bp-activity/css/mentions.css files in the /build` folder because the only difference is the RTL ignore comment, and comments are stripped from minified CSS and with that there are no differences in the minified CSS between CSSJanus and RTLCSS

Steps to finish up this ticket:

Apply 7007.1.patch and run grunt build, after that running svn diff or git diff should result in a patch identical to 7007-src.diff, commit the changes separately or together, chose your own adventure :)

This ticket was mentioned in Slack in #buddypress by dcavins. View the logs.


4 years ago

#5 follow-up: @dcavins
4 years ago

This looks good to me; I applied the patch and ran grunt build. @netweb is the expert on build tools. :)

I'll attach my resulting diff, and happily commit this change if I can get a sign-off from a lead.

#6 in reply to: ↑ 5 @netweb
4 years ago

Replying to dcavins:

This looks good to me; I applied the patch and ran grunt build. @netweb is the expert on build tools. :)

I'll attach my resulting diff, and happily commit this change if I can get a sign-off from a lead.

Awesome, thanks for testing, the changes in 7007.results-dc.patch look perfect.

#7 @DJPaul
4 years ago

  • Keywords commit added

If you've tested it and compared the outputs and they look the same, feel free to commit.

#8 @netweb
4 years ago

Have just double checked the results between 7007-src.diff and 7007.results-dc.patch and all is good, except for 1 difference though it relates to r10799 and is included in the 7007.results-dc.patch patch.

From the looks of it either the src/bp-members/admin/css/admin-rtl.css was created manually and the LTR width: 192px; was missed for the RTL where it is currently using width: 200px; or CSSJanus ignored it ¯\_(ツ)_/¯

Either/or and this is still fine for commit, just wanted to make mention of it and that change should also be committed :)

#9 @dcavins
4 years ago

In 10807:

RTL CSS generation: Move from CSSJanus to RTLCSS.

Moving to the grunt tool RTLCSS for right-to-left
CSS generation keeps us in step with WordPress.
RTLCSS also natively handles more properties than
CSSJanus.

See #7007.

Props netweb.

#10 @dcavins
4 years ago

In 10808:

Apply RTLCSS to stylesheets.

This commit records the updates to our CSS files
as rendered via the new grunt tool RTLCSS.

See #7007.

Props netweb.

#11 @dcavins
4 years ago

  • Resolution set to fixed
  • Status changed from new to closed

Thanks @netweb!

#12 follow-up: @hnla
4 years ago

Why does this not include the main stylesheets?

This ticket was mentioned in Slack in #buddypress by dcavins. View the logs.


4 years ago

#14 in reply to: ↑ 12 @netweb
4 years ago

Replying to dcavins:

Thanks @netweb!

:)

Replying to hnla:

Why does this not include the main stylesheets?

It does, it includes every BuddyPress stylesheet, that said it this only a tooling change for generating RTL CSS files and there are only a handful of RTL CSS changes and they are in r10808

#15 @DJPaul
4 years ago

  • Component changed from Tools - Build Process to Build/Test Tools
Note: See TracTickets for help on using tickets.