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

Fix missing thumbnails in mobile search #170

Merged
merged 1 commit into from
Jun 22, 2022
Merged

Conversation

jankrcal
Copy link
Member

This PR addresses a few minor issues in search:

  • fixes missing thumbnails on mobile (a new style introduced in Support podcast episodes in web-core #168 in search.scss was unintentionally applied only to desktop)
  • tweaks styling of the overlay text on mobile (making it smaller to better fit the small size of the thumbnail; especially important for the longer "Podcast 2050" overlay)
  • makes podcast open in a new window/tab from search (to make the behavior consistent with usages in boxes and elsewhere) - podcasts are detected by the URL being absolute, starting with a URL scheme like "https://"

@jankrcal
Copy link
Member Author

@katkolouchova Could you please take a look?

This is semi-urgent as it fixes a nasty visual bug in mobile search. Anytime tomorrow is okay. If you can't make it, please let me know!

This could be a complexity of a change that you could soon handle independently but feel free to disagree ;)

@katkolouchova
Copy link
Member

I've tested it locally and it seems to be working well. The only thing I'm a bit hesitant about is the overlay of "Podcast 2050" and episode number (see the screenshot below). Moving the episode number up would in my opinion look better, but perhaps this is something you've already discussed and agreed on with Petr? However, for the time being, I think it can be merged as it is now.

image

@jankrcal jankrcal merged commit 7d34aca into master Jun 22, 2022
@jankrcal jankrcal deleted the fix-mobile-search branch June 22, 2022 12:49
@jankrcal
Copy link
Member Author

Yeah, that sucks but I am working with Petr on a proper fix.

I don't want to delay this PR further.

@jankrcal
Copy link
Member Author

For the record, the episode number is part of the jpg image so I can't easily move it.
Irina has to create new assets for this use case (without episode numbers).

@katkolouchova
Copy link
Member

Yes, I've figured. Good to hear that you are already working on that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants