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

Shoreditch support: Fix following change in core that resulted in visual regression in shoreditch in advanced search #12524

Conversation

mukeshcompucorp
Copy link
Contributor

Overview

This PR for regression issues Advanced search.

Before

advanced search drupal demo

After

advanced search drupal demo 2

@civibot
Copy link

civibot bot commented Jul 20, 2018

(Standard links)

@eileenmcnaughton eileenmcnaughton changed the title SHOR-9: Fix regression issues advanced search SHOR-9: Fix change in core resulting in visual regression in shoreditch in advanced search Jul 20, 2018
@eileenmcnaughton
Copy link
Contributor

Given this merge on pass as we have added similar PRs adding line breaks before.

I guess just fixing things like this is OK - but there's nothing to stop it from keeping happening & nothing other than you reviewing PRs to pick it up.

The work Compucorp did here #12078 would be a more robust approach (although overkill for this fix). Not sure what else can be done to prevent layout issues affecting shoreditch - some clever test screenshotting regime? Some other html approach - dunno

@eileenmcnaughton eileenmcnaughton changed the title SHOR-9: Fix change in core resulting in visual regression in shoreditch in advanced search Shoreditch support: Fix following change in core that resulted in visual regression in shoreditch in advanced search Jul 20, 2018
@seamuslee001
Copy link
Contributor

Merging as per the tag

@seamuslee001 seamuslee001 merged commit bacf756 into civicrm:master Jul 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants