Make the sources tables responsive using CSS

[szymon-polaczy]

Fixes

Fixes #470 by @kajalmultidots

Description

This PR updates the source tables based on the design from #470.

It uses that design for the small devices but not phones.

On mobile phones it uses it as a baseline and changes the layout for better responsivity.

Concern was raised that this means that the table is really long on mobile devices but for now this was the best option.

Testing Instructions

Run the frontend and go to the /sources page. To see updated designs you have to check less than 768px width for the wide option and less than 640px for the tall option.

Checklist

• My pull request has a descriptive title (not a vague title likeUpdate 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 hope it does, I tried to cut down on the number of html elements by going with grid on tablets instead of using div elements with flex)
• 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.
• I ran the DAG documentation generator (./ov just catalog/generate-docs for catalog
  PRs) or the media properties generator (./ov just catalog/generate-docs media-props
  for the catalog or ./ov just api/generate-docs for the API) where applicable.

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.

[zackkrida]

Hi, @szymon-polaczy just checking in on the status of this PR. Need any help or reviews?

[szymon-polaczy]

Hi, @szymon-polaczy just checking in on the status of this PR. Need any help or reviews?

Not yet, I still need to update the images and then ping everyone that wanted to be pinged. I'll probably do that tomorrow

[fcoveram]

Feel free to ping me @szymon-polaczy once you have something ready for review.

[szymon-polaczy]

I'm making this ready for review without the updated snapshots for now as instructed @zackkrida.

@fcoveram I think you should be able to look at the changes now, though if you want to look at a whole changed snapshot instead of the website then you'll need to wait a bit more.

There is also another build going on because I rebased the PR to be up to date with main

[obulat]

@szymon-polaczy, I ran ./ov just frontend/run test:playwright -u locally, and committed the 4 updated snapshots.

@fcoveram, now that we've added the links to the sources pages on openverse to the sources table, maybe we could remove the domain column? The link to the source's site is available on the collection page (e.g., https://openverse.org/image/collection?source=inaturalist)

[szymon-polaczy]

Thanks again for the help with the snapshots @obulat .
If I could jump in I think that it would be a good idea to leave the domain as is.

On bigger screens the button that leads to the actuall site is quite small and easy to miss.

Having the domain on the table should also help in situations where someone actually wants to go to the main site and isn't just looking around as we would be saving them a page load

[obulat]

On bigger screens the button that leads to the actuall site is quite small and easy to miss.

This is another thing to consider. @fcoveram, do you think we should update the grid design for very large screens? It looks pretty empty. And we have many visitors on desktops.

[AetherUnbound]

@obulat It seems like we're waiting on some feedback from Francisco, should I wait for that before reviewing? Would it be best to draft until he has a chance to answer your questions?

[szymon-polaczy]

Hi @AetherUnbound @obulat, I think that it would be a good idea to go through with this as is (or maybe just change the domain as it's still the same element) and open a completely separate issue for the big / small screens. If needed I could also in that issue add some comments from me as I use a 4k screen as my main monitor, a macbook as seconday and iphone 13 mini as daily driver so I have a pretty good feel for all sizes and as a new user I could point out some places where it looks weird or out of place. What do you think about that?

[szymon-polaczy]

Also I'll make this a draft if we don't come up to a conclusion tomorrow to not have the bot ping the reviewers every day about it

[sarayourfriend]

I don't think this needs to be blocked on feedback from @fcoveram.

We don't need to remove the domain to fix the mobile table issue, do we? Why block the fix for mobile on that page on yet another change outside the scope of the original bug, which can certainly be done in a separate PR without increasing the difficulty of it, and which not doing now presents no harm to the page. Even if it does cause some problem (please document this if so), it's certainly not more harmful than the page just not working on mobile?

[sarayourfriend]

Is there a difference between hidden md:table and sm:hidden? Wondering mostly because sm:max-md:hidden avoids encoding an implementation detail about the template in the style, while also reducing the overhead of understanding when this is visible, in particular compared to the mobile table which is md:hidden.

[szymon-polaczy]

This is made this way as the normal table is hidden until you hit the md breakpoint, and the mobile table is visible until you hit the md breakpoint. If doesn't answer your question could you try to reword it? I'm trying to understand the question but I'm getting lost a bit as the classes don't really make sense to me

[sarayourfriend]

My comments are just nit-picks and none are blockers, just suggestions or questions. I will do a proper review for functionality in about an hour (unable to just this minute).

[sarayourfriend]

Okay, this tests well for the most part. I spent some time getting my Windows VM back up so I could test this with JAWS and there's at least one easy to fix accessibility issue. I've left a suggestion for it that works locally. Otherwise, I think this is fine.

@dhruvkb if you can take some time to test this with VoiceOver, that'd be great 🙏

I will be on holiday starting tomorrow, so please dismiss my review once the change is in and I'm sure others will be able to review this.

The only additional thing worth bringing up is that I thought we may have frozen the frontend code for the Nuxt 3 work, but how that work is progressing or what the plan is to get it merged and deployed is unclear to me. @obulat can you clarify if the frontend code is frozen and whether this PR can be merged or if it needs to wait until after Nuxt 3 (and then rebased and refactored if needed)? It'd be nice to get this in, considering it's been open for a month now.

Thanks @szymon-polaczy for your contribution and time for fixing this bug. The new mobile version looks very good!

[sarayourfriend]

I don't totally understand the usage of article. What is the intention meant to communicate with it? In JAWS it adds article start/end announcements when navigating through the page, without any useful context to know what the article would be about.

It would help to add title to this element. JAWS then announces the title instead of just "article" which makes it possible to skim the sources list by moving through each article instead of needing to go two elements into each article just to find out the name of the sources.

Suggested change

	<article v-for="provider in sortedProviders" :key="provider.display_name">
	<article v-for="provider in sortedProviders" :key="provider.display_name" :title="provider.display_name">

This seemed to test well locally when I tried it with JAWS.

[szymon-polaczy]

Thank you for showing me how you do the accessibility tests. I used the article element as in my head each row, each site was it's own box, not a full section, just an article. It seemed better than using div but it you think it might be better for accessibility then I'm open to chaning it

[fcoveram]

I added myself back as reviewer to approve what I'm seeing in my local. Great work ✨

[fcoveram]

Domain column in table

I'm drawn to keep the column to ease visiting the source site. Going to the source results page from the table to then visit the source site doesn't sound like a common flow.

Frozing the frontend

I approved the PR based on what I see on my browser, but I will defer to you all what's better in terms of implementation times.

Results grid

To @obulat's point

…do you think we should update the grid design for very large screens? It looks pretty empty. And we have many visitors on desktops.

Is that possible? I thought the number of results displayed on the page was a fixed value no matter the breakpoint. If it's doable, sure; I'd like to explore a solution.

---

@sarayourfriend's is right that my feedback should not be a blocker. If reviewers approve the changes, then any improvement skipped or missed can be addressed in a follow-up ticket. Apologies for not arriving timely to this work. Thanks @szymon-polaczy for the contribution.

[openverse-bot]

Based on the contributor urgency of this PR, the following reviewers are being gently reminded to review this PR:

@AetherUnbound
@dhruvkb
This reminder is being automatically generated due to the urgency configuration.

Excluding weekend¹ days, this PR was ready for review 6 day(s) ago. PRs labelled with contributor urgency are expected to be reviewed within 3 weekday(s)².

@szymon-polaczy, if this PR is not ready for a review, please draft it to prevent reviewers from getting further unnecessary pings.

Footnotes

1. Specifically, Saturday and Sunday. ↩

2. For the purpose of these reminders we treat Monday - Friday as weekdays. Please note that the operation that generates these reminders runs at midnight UTC on Monday - Friday. This means that depending on your timezone, you may be pinged outside of the expected range. ↩

[szymon-polaczy]

Hi @sarayourfriend, I've either implemented your comments or responded to them.

Should I try doing something with the snapshots @obulat? I see that you've forced pushed a change that removed them so I'm wondering if there's something that should be done with that or should I just recreate them

[obulat]

Hi @sarayourfriend, I've either implemented your comments or responded to them.

Should I try doing something with the snapshots @obulat? I see that you've forced pushed a change that removed them so I'm wondering if there's something that should be done with that or should I just recreate them

Hi @szymon-polaczy, sorry for the confusion caused by my not communicating anything. I was planning to approve your PR yesterday after using it to test the new snapshot update process, but I couldn't finish the work yesterday.
I tested the tables on Safari with VoiceOver, and the output is very clear.
Thank you for all of the work on this PR, and again, sorry for the confusion.

[obulat]

Thank you for your contribution and for improvements to accessibility, @szymon-polaczy !

Requested changes were added