Skip to content

Conversation

@roykho
Copy link
Contributor

@roykho roykho commented Sep 10, 2021

All Submissions:

Changes proposed in this Pull Request:

Closes #28996

So this issue was originally fixed already in this PR #29397

But somehow got regressed by this PR #29706

How to test the changes in this Pull Request:

  1. Install/activate jQuery Migrate Plugin so you can easily switch the two different jQuery versions for testing.
  2. Add an attributes filter widget to your shop. Use Dropdown as display type.
  3. Go to the shop page and click on the Select2 dropdown and ensure the dropdown shows correctly aligned and that you can select any of the attributes listed. Select an attribute and let page reload.
  4. Click on the dropdown again and ensure it expands correctly and you are able to select other attributes as well.
  5. Do the same test with and without the admin toolbar visible. You can change this in your user settings.
  6. Also do the same tests (3 to 6) with the dropdown going upwards when not enough room to show on the bottom.
  7. Now do the same tests again (3 to 6) with using older jQuery version by changing the settings in jQuery Migrate plugin. Ensure all is looking nice.
  8. Also check that the dropdown is showing nicely in checkout page (country selection).

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?

Changelog entry

Fix - offsets not calculated correctly sometimes on select2 dropdowns causing usability issues.

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

@roykho roykho requested a review from barryhughes September 10, 2021 02:04
@barryhughes
Copy link
Member

But somehow got regressed by this PR

Yeah...I think our copy of SelectWoo here may have been updated in situ a couple of times, and so fell out of sync with upstream. Then the referenced PR took the latest code from upstream and applied it here, overwriting the local changes.

Copy link
Member

@barryhughes barryhughes left a comment

Choose a reason for hiding this comment

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

LGTM.


Default jQuery

  • Filter widget, with toolbar, opening down: ✓
  • Filter widget, with toolbar, opening up: ✓
  • Filter widget, without toolbar, opening down: ✓
  • Filter widget, without toolbar, opening up: ✓
  • Country selector, with toolbar, opening down: ✓
  • Country selector, with toolbar, opening up: ✓
  • Country selector, without toolbar, opening down: ✓
  • Country selector, without toolbar, opening up: ✓

Legacy jQuery

  • Filter widget, with toolbar, opening down: ✓
  • Filter widget, with toolbar, opening up: ✓
  • Filter widget, without toolbar, opening down: ✓
  • Filter widget, without toolbar, opening up: ✓
  • Country selector, with toolbar, opening down: ✓
  • Country selector, with toolbar, opening up: ✓
  • Country selector, without toolbar, opening down: ✓
  • Country selector, without toolbar, opening up: ✓

@barryhughes barryhughes merged commit fb4162f into trunk Sep 10, 2021
@barryhughes barryhughes deleted the fix/28996 branch September 10, 2021 12:32
@github-actions github-actions bot added this to the 5.8.0 milestone Sep 10, 2021
@github-actions
Copy link
Contributor

Hi @barryhughes, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:

  • Add the status: needs changelog label
  • Add the status: needs testing instructions label

@barryhughes barryhughes added release: add changelog Mark all PRs that have not had their changelog entries added. [auto] release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Sep 10, 2021
@zhongruige zhongruige added testing instructions added and removed release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Sep 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release: add changelog Mark all PRs that have not had their changelog entries added. [auto]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

select2: Wrong top position when a toolbar is active

4 participants