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

Fix header row text overlapping search #1834

Merged
merged 3 commits into from Sep 13, 2023
Merged

Conversation

adamwoodnz
Copy link
Contributor

@adamwoodnz adamwoodnz commented Aug 30, 2023

Extends #1829
Closes #1808, #1813

Props: @cynthianorman

Uses columns for heading section layouts where there is an element to the right of the title, to avoid wrapping when titles are long.

Removes CSS which negatively affects the alignment, and fixes the styling of the search input button.

Screenshots

Lesson Plan archive

Desktop Tablet Mobile
localhost_8888_topic_extending-wordpress_(Desktop) localhost_8888_topic_extending-wordpress_(iPad) localhost_8888_topic_extending-wordpress_(Samsung Galaxy S20 Ultra)

Home

Desktop Tablet Mobile
localhost_8888_(Desktop) (1) localhost_8888_(iPad) localhost_8888_(Samsung Galaxy S20 Ultra)

Testing

Ideally all of the pages with templates changed should be checked at various screen sizes, you may need to create these pages or content for them

Check that the search input button is fixed and there is no white appearing below or above it

Splitting the row in 2 parts with the search field top right align and the heading title just below it left align.
modified the class for the row by using gutters instead of align-middle and using col-8 to restrict spacing of long titles
@@ -13,7 +13,7 @@

<main id="main" class="site-main">

<div class="row align-middle between section-heading section-heading--with-space">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These classes aren't required on headings where there is no right element, and cause the title to be centered on small devices, which is not desirable.

Comment on lines +23 to +28
<div class="section-heading section-heading--with-space row align-middle between gutters">
<?php the_archive_title( '<h1 class="section-heading_title h2 col-8">', '</h1>' ); ?>
<?php if ( is_user_logged_in() ) : ?>
<a class="section-heading_link" href="/my-courses/"><?php esc_html_e( 'My Courses', 'wporg-learn' ); ?></a>
<div class="col-4 row section-heading_links">
<a class="section-heading_link" href="/my-courses/"><?php esc_html_e( 'My Courses', 'wporg-learn' ); ?></a>
</div>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This pattern ensures there is space (gutters) between the elements both on desktop and smaller devices, and keeps the title left aligned, and any right elements right aligned within their column.

It's used for all the templates in this PR where the header uses this split layout.

@jonathanbossenger jonathanbossenger added the [Dev] Needs Review Pull request needing a review. label Aug 31, 2023
@jonathanbossenger
Copy link
Collaborator

Thank you @adamwoodnz I'll make sure to do a more thorough review this time around.

Copy link
Collaborator

@jonathanbossenger jonathanbossenger left a comment

Choose a reason for hiding this comment

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

@adamwoodnz this all looks great in my testing. Thanks for wrangling this.

@adamwoodnz adamwoodnz merged commit 4496525 into trunk Sep 13, 2023
1 check passed
@adamwoodnz adamwoodnz deleted the 1808-text-overlapping branch September 13, 2023 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Dev] Needs Review Pull request needing a review.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Long Topic Title in Learn WordPress overlapping with the searchbox
3 participants