Skip to content

Add RTL admin-bar.css and update fork to WP 5.3-alpha@160fc055da#2972

Merged
westonruter merged 5 commits intodevelopfrom
update/admin-bar-css
Aug 7, 2019
Merged

Add RTL admin-bar.css and update fork to WP 5.3-alpha@160fc055da#2972
westonruter merged 5 commits intodevelopfrom
update/admin-bar-css

Conversation

@westonruter
Copy link
Copy Markdown
Member

@westonruter westonruter commented Aug 6, 2019

Add RTL admin-bar

This PR includes an admin-bar-rtl.css and ensures it gets loaded when is_rtl().

Before

Screen Shot 2019-08-06 at 13 58 57

After

Screen Shot 2019-08-06 at 14 15 01

Update Forks

The admin-bar.css in core has been updated since we forked it from WP 4.9. This updates it to the latest: https://github.com/WordPress/wordpress-develop/blob/160fc055da156248277513dd5256fbcc66faa5a7/src/wp-includes/css/admin-bar.css

Changes applied to fork:

0a1,10
> /*
> This is forked from core's admin-bar.css from WP 5.3-alpha at 160fc055da156248277513dd5256fbcc66faa5a7.
> 
> https://github.com/WordPress/wordpress-develop/blob/160fc055da156248277513dd5256fbcc66faa5a7/src/wp-includes/css/admin-bar.css
> 
> - References to .hover have been replaced with :focus-within (which is not supported in IE11).
> - Universal selector properties have been removed which interferes with AMP shadow elements.
> */
> 
> 
6c16
< 	position: static;
---
> 	/* Removed because interferes with amp-img>img: position: static; */
187c197
< #wpadminbar .quicklinks .menupop.hover ul li .ab-item,
---
> #wpadminbar .quicklinks .menupop:focus-within ul li .ab-item,
201c211
< #wpadminbar li.hover > .ab-sub-wrapper {
---
> #wpadminbar li:focus-within > .ab-sub-wrapper {
206c216
< #wpadminbar .menupop li.hover > .ab-sub-wrapper {
---
> #wpadminbar .menupop li:focus-within > .ab-sub-wrapper {
212c222
< #wpadminbar .ab-top-secondary .menupop li.hover > .ab-sub-wrapper {
---
> #wpadminbar .ab-top-secondary .menupop li:focus-within > .ab-sub-wrapper {
221c231,235
< #wpadminbar .ab-top-menu > li.hover > .ab-item {
---
> #wpadminbar .ab-top-menu > li:focus-within > .ab-item {
> 	background: #32373c;
> 	color: #00b9eb;
> }
> #wpadminbar .ab-top-menu > li:focus-within > .ab-item {
227c241
< #wpadminbar > #wp-toolbar li.hover span.ab-label,
---
> #wpadminbar > #wp-toolbar li:focus-within span.ab-label,
273c287
< #wpadminbar .quicklinks .menupop.hover ul li a,
---
> #wpadminbar .quicklinks .menupop:focus-within ul li a,
283,287c297,301
< #wpadminbar .quicklinks .ab-sub-wrapper .menupop.hover > a,
< #wpadminbar .quicklinks .menupop.hover ul li a:hover,
< #wpadminbar .quicklinks .menupop.hover ul li a:focus,
< #wpadminbar .quicklinks .menupop.hover ul li div[tabindex]:hover,
< #wpadminbar .quicklinks .menupop.hover ul li div[tabindex]:focus,
---
> #wpadminbar .quicklinks .ab-sub-wrapper .menupop:focus-within > a,
> #wpadminbar .quicklinks .menupop:focus-within ul li a:hover,
> #wpadminbar .quicklinks .menupop:focus-within ul li a:focus,
> #wpadminbar .quicklinks .menupop:focus-within ul li div[tabindex]:hover,
> #wpadminbar .quicklinks .menupop:focus-within ul li div[tabindex]:focus,
295,296c309,310
< #wpadminbar li.hover .ab-icon:before,
< #wpadminbar li.hover .ab-item:before,
---
> #wpadminbar li:focus-within .ab-icon:before,
> #wpadminbar li:focus-within .ab-item:before,
307,308c321,322
< #wpadminbar.mobile .quicklinks .hover .ab-icon:before,
< #wpadminbar.mobile .quicklinks .hover .ab-item:before {
---
> #wpadminbar.mobile .quicklinks :focus-within .ab-icon:before,
> #wpadminbar.mobile .quicklinks :focus-within .ab-item:before {
395c409
< #wpadminbar .ab-top-menu > #wp-admin-bar-recovery-mode.hover >.ab-item,
---
> #wpadminbar .ab-top-menu > #wp-admin-bar-recovery-mode:focus-within >.ab-item,
482c496
< 	width: auto;
---
> 	width: 16px; /* Was auto. */
491c505
< 	display: inline;
---
> 	display: inline-block; /* Was inline. */
496c510
< 	width: auto;
---
> 	width: 16px; /* Was auto. */
532c546
< #wpadminbar .quicklinks .ab-sub-wrapper .menupop.hover > a .blavatar {
---
> #wpadminbar .quicklinks .ab-sub-wrapper .menupop:focus-within > a .blavatar {
855c869
< 	#wpadminbar .quicklinks .menupop.hover ul li .ab-item,
---
> 	#wpadminbar .quicklinks .menupop:focus-within ul li .ab-item,
866c880
< 	#wpadminbar .menupop li.hover > .ab-sub-wrapper {
---
> 	#wpadminbar .menupop li:focus-within > .ab-sub-wrapper {
1060c1074
< 	#wpadminbar li.hover ul li,
---
> 	#wpadminbar li:focus-within ul li,
1109,1111c1123
< 	#wpadminbar {
< 		position: absolute;
< 	}
---
> 	/* Removed: #wpadminbar { position: absolute; } */

@westonruter westonruter added this to the v1.2.1 milestone Aug 6, 2019
@googlebot googlebot added the cla: yes Signed the Google CLA label Aug 6, 2019
@westonruter westonruter force-pushed the update/admin-bar-css branch from 39c266d to 2ffc5df Compare August 6, 2019 20:40
@westonruter westonruter changed the title Update forked admin-bar.css to WP 5.3-alpha@160fc055da Add RTL admin-bar.css and update fork to WP 5.3-alpha@160fc055da Aug 6, 2019
@westonruter westonruter requested a review from swissspidy August 6, 2019 21:21
@westonruter westonruter marked this pull request as ready for review August 6, 2019 21:21
@swissspidy
Copy link
Copy Markdown
Collaborator

swissspidy commented Aug 6, 2019

Can we automate this? If I‘m not mistaken we already use rtlcss for other styles.

@westonruter
Copy link
Copy Markdown
Member Author

Yeah, we should. I don't know how to configure that, however. Something like this?

// ...
const adminBar = {
	...defaultConfig,
	module: {
		rules: [
			{
				test: /admin-bar\.css$/,
				use: [
					MiniCssExtractPlugin.loader,
					'css-loader',
					'postcss-loader',
				],
			},
		],
	},
	plugins: [
		new MiniCssExtractPlugin( {
			filename: '../css/[name]-compiled.css',
		} ),
		new RtlCssPlugin( {
			filename: '../css/[name]-compiled-rtl.css',
		} ),
		new WebpackBar( {
			name: 'Admin Bar',
			color: '#67b255',
		} ),
	],
};
// ...

@swissspidy
Copy link
Copy Markdown
Collaborator

At first glance that seems like a step in the right direction 👍 I can have a closer look tomorrow.

@westonruter
Copy link
Copy Markdown
Member Author

Yeah, except it doesn't work 😄

@swissspidy
Copy link
Copy Markdown
Collaborator

As for the webpack config, entry and output were missing. But even then, I had issues with the CSS loader processing it.

Given that webpack does not yet support standalone CSS processing very well (it always builds a JS chunk), it's probably easiest to just use RTLCSS standalone for this.

Since we need to add RTL versions for all other stylesheets anyway, I suggest exploring this in a separate issue. Then we can also look into using PostCSS for all these stylesheets, which allows us to use things like postcss-themes as well.

One thing that I already noticed: In order to not override the wrong files or create too many files, we should separate assets/css into a src and dist folder, like we already have for JS.
Right now we have kind of a mix in this folder (see *-compiled.css files).

@westonruter westonruter merged commit cd34604 into develop Aug 7, 2019
@westonruter westonruter deleted the update/admin-bar-css branch August 7, 2019 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Signed the Google CLA

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants