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

translate.w.org social icons aren't on the same line #78

Closed
3 tasks done
Tracked by #44
iandunn opened this issue Jan 12, 2022 · 6 comments
Closed
3 tasks done
Tracked by #44

translate.w.org social icons aren't on the same line #78

iandunn opened this issue Jan 12, 2022 · 6 comments
Assignees
Labels
[Type] Bug Something isn't working
Milestone

Comments

@iandunn
Copy link
Member

iandunn commented Jan 12, 2022

this happened before because the wp-container-{hash} styles weren't being output, but that was fixed by f5835d2

Screen Shot 2022-01-11 at 5 03 46 PM

@iandunn iandunn self-assigned this Jan 12, 2022
@iandunn iandunn added the [Type] Bug Something isn't working label Jan 12, 2022
@iandunn iandunn added this to the News launch milestone Jan 12, 2022
@iandunn iandunn changed the title translate has social icons not on same line. this happened before because the wp-container-{hash} styles weren't being output, but that was fixed by f5835d2 translate.w.org social icons aren't on the same line Jan 12, 2022
@iandunn
Copy link
Member Author

iandunn commented Jan 12, 2022

That is happening because:

  1. the ul doesn't have display: flex, which is because...
  2. the wp-container-{hash} styles from gutenberg_render_layout_support_flag() aren't output, which is because...
  3. that function hooks them to wp_footer, but that's not called in glotpress, which is because...
  4. GP ignores the activated theme and uses it's built-in one instead. only gp_footer is called, not wp_footer

I suspect the view.min.js issue is a similar problem w/ wp_enqueue_script() vs gp_enqueue_script().

I'm thinking of doing something like the following, but it's obviously a fragile hack.

global $wp_filter;
foreach ( $wp_filter['wp_footer'][10] as $callback ) {
	$reflected          = new \ReflectionFunction( $callback['function'] );
	$is_layout_file     = false !== strpos( $reflected->getFileName(), 'block-supports/layout.php' );
	$has_style_variable = isset( $reflected->getStaticVariables()['style'] );

	if ( $is_layout_file && $has_style_variable ) {
		add_action( 'gp_footer', $callback );
	}
}

@ryelle , @dd32, @ocean90 : Do any of y'all see a better way?

@dd32
Copy link
Member

dd32 commented Jan 12, 2022

Another aspect of this is that GlotPress doesn't accept that scripts/styles might be printed in the footer (Which might be why view.min.js isn't loading?) as it only adds head-related hooks .

At least on WordPress.org, I think the solution is to drop the GlotPress header/footer calls, and hook them to the WordPress actions instead. It would mean some hackery, but I feel like it's likely to be less hackery than the above.
GlotPress/GlotPress#8 is the upstream GlotPress ticket for migrating away from the GlotPress specific template structure.

@iandunn
Copy link
Member Author

iandunn commented Jan 12, 2022

Yeah, that does sound better. I think it might also help to create a blank/empty theme, and activate that on translate.w.org, so that TwentyFifteen scripts/styles aren't enqueued.

I'll play around w/ all that.

bazza pushed a commit to WordPress/wordpress.org that referenced this issue Jan 12, 2022
The block now only calls `wp_head|footer()`, and isn't aware of GlotPress, so these templates need to load things that are unique to GlotPress.

See WordPress/wporg-mu-plugins#78


git-svn-id: https://meta.svn.wordpress.org/sites/trunk@11435 74240141-8908-4e6f-9713-ba540dce6ec7
@iandunn
Copy link
Member Author

iandunn commented Jan 14, 2022

We'll probably need to revert part of those two commits, and move gp_head|footer() back to classic-header|footer.php, so that they get output inside <head>.

I guess the footer could still be in GlotPress templates, but then it wouldn't be consistent 🤷🏻

We also had to re-enable TwentyFifteen for the current site, so that will need to be addressed. One option might just be to swap the theme when we launch the new header.

@iandunn iandunn reopened this Jan 14, 2022
@ryelle
Copy link
Contributor

ryelle commented Jan 14, 2022

Is the issue here still the social icons? Because I see a lot more wrong right now:

Screen Shot 2022-01-14 at 17 41 18

@iandunn
Copy link
Member Author

iandunn commented Jan 14, 2022

That's because TwentyFifteen was reactivated, and its stylesheets are enqueued again. I'm working on it now.

bazza pushed a commit to WordPress/wordpress.org that referenced this issue Jan 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants