Add Dark mode support to listings and also fix functionality to next/previous buttons #258
Conversation
It may not be a bad idea to - add a placeholder image/text when none is present for a listing (even something as simple as "No Image" to make it clear that this was intentional from the part of the listing creator, and not a network error) - remove the `animate-pulse` effect in dark mode; I hesitantly kept it in for now but it is plausible that animate-pulse loses its magic in dark mode.
chennisden
force-pushed
the
251-listing-dark-mode
branch
from
5036898 to
43a8f86
Compare
- the previous and next buttons took people to /browse/PAGENUMBER rather than /browse/?page=PAGENUMBER, where the frontend expects the latter. Fixed. - having fixed that, when we go to a new page, the listings do not refresh because the useEffect calling handleSearch has never been called; the useEffect hook now updates when pageNumber changes.
chennisden
force-pushed
the
251-listing-dark-mode
branch
from
43a8f86 to
c8627a8
Compare
|
The GitHub action history is a bit of a mess because I screwed up rebasing. All is good now. Here's a screenshot. Before: |
| <div> | ||
| <div className="animate-pulse bg-gray-200 w-full aspect-square"></div> | ||
| </div> | ||
| <div className="animate-pulse bg-gray-200 dark:bg-gray-800 w-full aspect-square" /> |
There was a problem hiding this comment.
i think there was a reason i wasn't doing this
There was a problem hiding this comment.
I'm going to assume it's because backgrounds might be white
There was a problem hiding this comment.
I think either way dark mode listings is going to look a little silly; it's just a question of which looks more silly. A question of personal taste, I suppose.
There was a problem hiding this comment.
oh no, it had to do with getting it to actually be a square - if i removed it then the div didn't render as a square correctly
There was a problem hiding this comment.
With #261 I think this is by far the right thing to do.
|
why does the image in your screenshot look messed up? |
Fixed in a separate PR. This is a pre existing issue |
| <div> | ||
| <div className="animate-pulse bg-gray-200 w-full aspect-square"></div> | ||
| </div> | ||
| <div className="animate-pulse bg-gray-200 dark:bg-gray-800 w-full aspect-square" /> |
There was a problem hiding this comment.
oh no, it had to do with getting it to actually be a square - if i removed it then the div didn't render as a square correctly
Resolves #251
The commit messages are self-explanatory.