Fixed overlap between button text and icon

[Realmbird]

Closes #8899

Fixes overlap between button text and icon

Technical

• Issue: Audio book read buttons had button text & icon "external link" overlap on smaller screen sizes

• Cause: Responsive booksPerBreakpoints were not accurate with when less pages should be shown on the page

• Fix: Changing responsive booksPerBreakpoints was not css like issue had stated

Testing

1. docker compose up
   // For running site
2. docker compose exec -e PYTHONPATH=. web bash -c "./scripts/copydocs.py --search 'author_key:OL9411725A' --search-limit 100"
   // Adding data
3. http://localhost:8080/works/OL36891831W/
4. Start changing screen width

Screenshot

Screencast from 04-10-2024 01:54:07 PM.webm
Will be screencast to demonstrate that text never overlaps with icon

Stakeholders

@cdrini @RayBB

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 15.93%. Comparing base (ae62bd8) to head (2edaa06).
Report is 16 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #9066   +/-   ##
=======================================
  Coverage   15.93%   15.93%           
=======================================
  Files          89       89           
  Lines        4720     4720           
  Branches      823      823           
=======================================
  Hits          752      752           
  Misses       3457     3457           
  Partials      511      511           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[RayBB]

@Realmbird it is on testing now and it is working as expected. I think it fixes the initial concern. However, I want to run it by the design team in the call today. 👍

[Realmbird]

Thanks

[RayBB]

The design call didn't have time for me to bring this up.

Maybe @cdrini can comment directly.

[RayBB]

@Realmbird we have to wait for @cdrini to comment on this one. He's off Thursday and Friday so it will probably have to wait until next week.

Here are my thoughts though:

• Your PR does fix the issue
• However, I'm not sure if those numbers you changed in the JS were set for a specific reason so I'm hesitant to change them.
• In my opinion, I think it is a better long-term fix to move the image out of the background and have it be right next to the text. But that's just my opinion and changes the design of the button. I assume the original goal was that the external icon stays to the far right while the text stays centered. I'm not sure how attached we are to that design.

[RayBB]

@Realmbird
I asked in the design call today.
The feedback I got from @mekarpeles was that maybe we should see if we can make the text/icon scale smaller with the size of the button. We are still not sure if that is feasible and probably want to get feedback from @cdrini before spending too much time on that path.

[Realmbird]

Problem with text and icon is current version is a background image for the button, so it can't be scaled well currently

[RayBB]

@Realmbird feel free to play around with moving the icon out but ultimately we should wait for Drini's input here. We can try to get his input on the Tuesday call.

[Realmbird]

Ok in the mean time gonna work on other issue

[cdrini]

Howdy folks! I think what we can try is; apologies it will be a little involved!

1. Decrease the font size of all cta-btn from 16px to 14px. Make only the primary button, the one inside the #read-options element, be 16px.
2. Make all .cta-btn white-space: nowrap; overflow: hidden; text-overflow: ellipsis; and make .cta-btn--external have a padding-right of 25px ; this should make sure there's enough space for the external icon, and crop the text with an ellipsis if there isn't
3. Make all buttons have padding: 8px 0 instead of the current padding: 8px ; the button label already has padding, so we're just doubling up on it.
4. Make all buttons have text-align: center; this is getting lost somewhere for carousels.

That should do it, and give us something like this

Here's the css I experimented with ; although yours will likely look different / be in a different spot

[Realmbird]

Ok will make changes to css

[Realmbird]

@cdrini Added your changes

[Realmbird]

Screencast from 05-22-2024 11:22:57 AM.webm
After changes

[RayBB]

Suggested change

	responsive: [1200, 1024, 740, 580, 360]
	responsive: [1200, 1024, 600, 480, 360]

Lets restore these (I think that's what Drini wanted) then I'll put it on testing so we can see how things look!

[Realmbird]

Changing numbers breaks it on firefox it briefly overlaps

[Realmbird]

@RayBB @cdrini Tried rays changes caused overlap bug to reappear

[Realmbird]

[RayBB]

@Realmbird that screenshot didn't finish uploading.
@cdrini how do you feel about the PR as is with the responsive size numbers changing?

[RayBB]

Closing this to offer up someone else to work on this one.