dev/wp#46 - Fix overlarge select2 elements in WordPress 5.3+

[wmortada]

Overview

Changes in the visual look of WordPress 5.3 have caused some select2 elements too appear larger than they should.

The issue is that WordPress sets a minimum height on input elements. I have fixed this issue by unsetting the minimum height.

See issue #46 for more details.

Before

A section of the Advanced Search form in WordPress 5.3. Note the height of the 'Select Tags' field is inconsistent with other elements.

After

The same section after this change.

Technical Details

Screenshots taken in Firefox on Ubuntu 18.04. Other browser/OS combinations may vary.

I have tested this (and #16882) in the following OS/browsers:

• Windows 10: Chrome, Firefox, Edge, IE
• Android: Chrome, Samsung Internet
• Ubuntu: Chrome, Firefox

And the following CMSs:

• Drupal 7
• Drupal 8
• WordPress 5.2
• WordPress 5.4
• Backdrop

Help testing in Safari and Chrome on Mac OS and iOS would be appreciated. Ditto for Joomla.

Comments

See related PR #16882 for an issue with the select element in WordPress 5.3.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[jaapjansma]

test this please

[wmortada]

I've been doing lots of testing of this @jaapjansma 😃

[demeritcowboy]

He just meant jenkins, to rebuild the test site here. But there's something going on where all PR tests are failing right now.

[wmortada]

Yes, I guessed that! 😃

[demeritcowboy]

Ah I misunderstood the joke.

Regarding the PR, in drupal 8 (and 7) that particular field looks wrong both before and after because the box is not aligned vertically with the rest of the row, and after there is still a min-height from .select2-choices. So, no change. That's both firefox and chrome.

And it looks like the test issue might be resolved so (jenkins) test this please.

[wmortada]

Hmm. I didn't notice that in my testing, but that sounds like a separate issue.

[wmortada]

In conjunction with #16882, I've tested this in a variety of combinations of OS/CMS/Browser (32 combinations to be precise!):

• Drupal 7
• Drupal 8
• WordPress 5.2
• WordPress 5.4
• Backdrop

on Windows / Ubuntu / Android
in Firefox / Chrome / Samsung Internet / IE / Edge

This change doesn't appear to cause any issues for other CMS. There is no notable difference in any of the combinations that I have tested.

[jaapjansma]

test this please

[jaapjansma]

@demeritcowboy when @wmortada starts joking again it is because we are doing reviews of pr again and we want to review this pr and we need test environment.

[wmortada]

Haha! Thanks @jaapjansma!

[jaapjansma]

• General standards
  • Explain (r-explain)
    • PASS : The goal/problem/solution have been adequately explained in the PR.
  • User impact (r-user)
    • PASS: The change would be intuitive or unnoticeable for a majority of users who work with this feature.
  • Documentation (r-doc)
    • PASS: The changes do not require documentation.
  • Run it (r-run)
    • PASS: We created a local test environment and compared it to wpmaster.demo.civicrm.org and it looks good. We have also checked the Drupal 7 environment and nothing seems to be broken on the advanced search screen.
• Developer standards
  • Technical impact (r-tech)
    • PASS: The change preserves compatibility with existing callers/code/downstream.
  • Code quality (r-code)
    • PASS: The functionality, purpose, and style of the code seems clear+sensible.
  • Maintainability (r-maint)
    • PASS: The change sufficiently improves test coverage, or the change is trivial enough that it does not require tests.
  • Test results (r-test)
    • PASS: The test results are all-clear.

@eileenmcnaughton or @mattwire could one of you merge this PR? @wmortada thanks for testing yourself in different browsers and cmses!

[mattwire]

test fail unrelated