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

Remove margin top from filter summary component #3675

Merged
merged 1 commit into from
Feb 21, 2025

Conversation

JamesCGDS
Copy link
Contributor

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Follow these steps if you are doing a Rails upgrade.

What

This PR removes margin-top: 10px from the filter summary component and adds margin-bottom: 10px on the filter panel component. This should not result in any visual differences as the filter summary always renders below the filter panel and the original 10px of spacing is maintained - the only difference is in how it is applied.

Why

Trello card

Visual changes

Hopefully none but these are some pages that can be checked:

Local / live (only filter panel rendered)
Local / live (filter panel rendered with filter summary)

In order to maintain spacing, I've added a 'margin-bottom: 10px' to the filter panel component. This should not result in any visual differences as the filter summary always renders beneath the filter panel.
@JamesCGDS JamesCGDS requested a review from AshGDS February 21, 2025 13:01
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3675 February 21, 2025 13:02 Inactive
Copy link
Contributor

@AshGDS AshGDS left a comment

Choose a reason for hiding this comment

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

EDIT: Ignore this!

@JamesCGDS JamesCGDS merged commit 7b903e3 into main Feb 21, 2025
13 checks passed
@JamesCGDS JamesCGDS deleted the remove_margin_top_from_filter_summary branch February 21, 2025 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants