fix margin bottom of pages menu

[anton202]

Fixes

Fixes #2008 by @sarayourfriend

Description

This PR fixes the absence of bottom margin in pages menu.

Testing Instructions

search for something. then, in the results page at the top of the page click on the 3 vertical dots button.
hover over the last element in the menu

Checklist

• My pull request has a descriptive title (not a vague title like Update index.md).
• My pull request targets the default branch of the repository (main) or a parent feature branch.
• My commit messages follow best practices.
• My code follows the established code style of the repository.
• I added or updated tests for the changes I made (if applicable).
• I added or updated documentation (if applicable).
• I tried running the project locally and verified that there are no visible errors.

Developer Certificate of Origin

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

[sarayourfriend]

@anton202 Thanks for the PR. It looks like the visual regression tests need updating for this change. If you run pnpm test:playwright -- visual-regression -u locally it will generate new versions of the snapshots that you can commit and push for review.

[obulat]

This PR fixes the issue, thank you, @anton202! I think if you re-base it onto main, you will stop getting CI errors due to flaky tests (I hope it's fixed in main now).

[anton202]

weird, there is no visual regression updates...
and i did the rebase also :)

[sarayourfriend]

@anton202 Looking at the screenshot you shared in the PR description, there is a right side margin added in this PR. However, when I run the changes locally I don't see that regression. Is the screenshot from a previous commit, by any chance?

[sarayourfriend]

One more question, sorry. Can you confirm how you're running the visual regression tests, what command you're using to execute them? When I run them locally I get failing visual regression tests. When I check the diffs, it looks like the new bitmaps committed in this PR are the issue and that the actual output matches the original expected output before this PR.

The two differences I'm seeing are:

1. The underlining of the arabic text of the RTL homepage at the text at the bottom is slightly different:
   

2. The search bar in the new snapshots added in this PR is taller than the expected (and what I get locally when I run the tests):
   

By the way, I pulled these from the test-results directory on my local machine after running the tests. You can check out our brief introductory guide to debugging our playwright tests here:

openverse-frontend/test/playwright/README.md

Line 85 in 59d9c39

## Debugging

To confirm, the command I'm using to run them locally is:

pnpm test:playwright -- visual-regression

When I check, it is using version v1.28.1. You can see the version being used logged after a note about successfully saving English translations when you run test:playwright.

[anton202]

hi @sarayourfriend
regarding the margin in the screenshot, i actually dont get this margin when running the project on a macbook air 2022.
this screen shot was taken from a 15 inch hp laptop... also i get this margin on a 24 inch dell display. same happens on staging open-verse web site.

macbook air runnig the project locally

.

15 inch hp laptop running the project locally

.

15 inch hp laptop. the current state of the pages menu on staging:

[anton202]

regarding the tests.
I run pnpm test:playwright visual-regression -u to update the snapshots,
and pnpm test:playwright visual-regression to run the visual regression tests.
iv'e rested the HEAD locally to the previous commit 7420610 basically removing the "updated snapshots" and now it seems that all tests pass.
i run the tests 3 times and they have passed....
is that ok to reset the head of this remote branch to the previous commit?

(Running Playwright v1.28.1 as 501 with Playwright arguments visual-regression)

[dhruvkb]

@anton202 it's definitely OK to reset the branch and/or rewrite its commit history as long as it's a work in progress. I would also recommend drafting the PR while it's not ready for review.

[anton202]

got it. thx @dhruvkb
reset the head to a commit before the updated snapshots.
we can try to run the test again

[sarayourfriend]

@anton202 Thanks for looking into the weird margin issue. If it's not a regression of part of this PR then it sounds good. Can you open an issue with what you've described, so someone can debug it? Maybe it's a platform or browser version issue, but it seems strange 🤔

[obulat]

I've re-based your PR onto the main branch to fix the Playwright tests flakiness. Sorry about that, the flakiness was introduced in one of the commits when you opened the PR, @anton202, and then it was fixed in main.

The changes look great, thank you for the quick fix! 🌟

[anton202]

@sarayourfriend is creating an issue for this weird margin issue is relevent? i mean isn't there a plan to release a new header soon, which won`t have this pages menu?

[zackkrida]

I'm honestly not understanding all of the discussions in the PR comments, but the changes look correct locally and the code is a simple change that matches the visual result. LGTM.