Conversation
| ); | ||
| } | ||
|
|
||
| seeAllBtn() { |
There was a problem hiding this comment.
This shouldn't be necessary; there is already an All Platforms option in the dropdown.
There was a problem hiding this comment.
The list is filtered by default now, and for someone who sees it for the first time, it may not be obvious where to change the filters. Especially if you scroll down a bit and dont find what youre looking for. Better to have an explicit button than having the user close the page.
There was a problem hiding this comment.
Then add it to the filterAndTitleBlock since it is a filter. Probably to the left of the dropdown.
There was a problem hiding this comment.
That would also work. But the idea is that you open the page, scroll down the list, and at the end notice that it doesnt have what youre looking for. At that point you might not think of scrolling all the way up again, and its very convenient if you can directly "show all". I also made it like this for the instance list (https://github.com/LemmyNet/joinlemmy-site/pull/534/files#diff-64ddea6fc06e051736ae2ea16ee2d1378833a0b0758e9ea701bd84bd4300f312R525).
There was a problem hiding this comment.
Alright I spose that's fine.
| <div className="flex justify-center p-8"> | ||
| <button | ||
| className="btn btn-secondary text-white normal-case" | ||
| onClick={linkEvent(this, handleSeeAll)} |
There was a problem hiding this comment.
I'm trying to move away from all linkEvent, because it calls functions without any parameters in an untyped way. Rust would not let us get away with stuff like this but unfortunately typescript does.
Better to do:
| onClick={linkEvent(this, handleSeeAll)} | |
| onClick={() => handleSeeAll(this)} |
There was a problem hiding this comment.
Done, although its used in lots of places and needs a separate cleanup PR then. We should also enable more lints for this repo.
There was a problem hiding this comment.
This still needs to be done I think.
2 participants
This is quite tricky, and I dont have a good way to test all of it.
It would also be neat if
DesktopshowedWebapps as there are very few desktop apps available right now. The obvious solution would be to turnstate.platforminto an array, but that gets awkward to manage.Requires #517